Stored Procedure code shortening and optimization

Question

This stored procedure takes user defined table type as parameter. It contains data imported from an Excel file. Values inside that type are the same as in #InconsistentRestriction temporary table. It's working fine, but since I'm not an SQL expert I'm afraid it's not written the best way possible (or not even close).

Combination of TextRestrictionTypeId, SMSChannelId, NetworkId, GatewayId and CountryId is unique and is used to recognize whether record with that unique key already exists inside dbo.SMSChannelNetworkTextRestrictions table or not. If record with such unique key already exists we're checking for consistency. All values with the same unique key must have the same PassIfMatch (NULL is considered same as FALSE when checking) value otherwise they're considered inconsistent and should not be inserted, but rather returned to user to correct them and reinsert them. Once you find a record with that unique key inside table, you should retrieve it's Id and insert combination of Id and Expression into dbo.TextRestrictionData table.

First, I'm creating a temporary table which contains the same columns as my user defined table type su.TextRestriction_Type. After that I'm joining user defined table type input with dbo.SMSChannelNetworkTextRestrictions table on unique key values and checking where PassIfMatch is not the same for input records and existing record. That way I'm getting the set of inconsistent input records and I'm placing them into temporary table.

Next step is the insertion of non existing unique key into dbo.SMSChannelNetworkTextRestrictions. So I'm joining input user defined table type with dbo.SMSChannelNetworkTextRestrictions and #InconsistentRestrictions using left join together with where condition to remove existing unique keys and unwanted restrictions from initial input user defined table type. Also, these record are grouped by because only one value of unique key combination can be inserted. By doing everything by now, I made sure every new expression got text restriction id existing inside dbo.SMSChannelNetworkTextRestrictions table and I'm doing expression insert into dbo.TextRestrictionData by selecting Expression from input table and Id from dbo.SMSChannelNetworkTextRestrictions table, also joining once again to #InconsistentRestriction temporary table to skip those invalid records when inserting expressions.

Lastly, I'm selecting inconsistent restrictions from that temp table in order to return them back to user to fix them. Hopefully I explained everything I needed to explain in order to help you understand my problematic.

Now, I'd appreciate if any of you took time to tell me if there is a way to shorten that code, maybe reduce those massive joins or even optimize what I'm doing, because these excels can get pretty massive amount of records.

ALTER PROCEDURE [su].[TextRestrictions_Multiple_Add]
 @TextRestrictionList su.TextRestriction_Type READONLY
AS
BEGIN
BEGIN TRANSACTION
BEGIN TRY
 CREATE TABLE #InconsistentRestrictions
 (
 TextRestrictionTypeId INT NOT NULL,
 [Enabled] BIT NOT NULL,
 SMSChannelId INT NULL,
 NetworkId INT NULL,
 GatewayId INT NULL,
 CountryId INT NULL,
 Expression NVARCHAR(100) NOT NULL,
 PassIfMatch BIT NULL
 );
 -- INSERTING INCONSISTENT INPUT RECORDS TO TEMPORARY TABLE (THOSE THAT HAVE PassIfMatch DIFFERENT THAN EXISTING MATCHING TABLE RECORD) --
 INSERT INTO #InconsistentRestrictions
 ( TextRestrictionTypeId, [Enabled], SMSChannelId, NetworkId, GatewayId, CountryId, Expression, PassIfMatch )
 SELECT trl.TextRestrictionTypeId
 ,trl.[Enabled]
 ,trl.SMSChannelId
 ,trl.NetworkId
 ,trl.GatewayId
 ,trl.CountryId
 ,trl.Expression
 ,trl.PassIfMatch
 FROM 
 @TextRestrictionList trl
 JOIN 
 dbo.SMSChannelNetworkTextRestriction tr
 ON 
 tr.TextRestrictionTypeId = trl.TextRestrictionTypeId
 AND (tr.SMSChannelId = trl.SMSChannelId OR ISNULL(tr.SMSChannelId, trl.SMSChannelId) IS NULL)
 AND (tr.NetworkId = trl.NetworkId OR ISNULL(tr.NetworkId , trl.NetworkId ) IS NULL)
 AND (tr.GatewayId = trl.GatewayId OR ISNULL(tr.GatewayId , trl.GatewayId ) IS NULL)
 AND (tr.CountryId = trl.CountryId OR ISNULL(tr.CountryId , trl.CountryId ) IS NULL)
 WHERE
 ISNULL(tr.PassIfMatch, 0) != ISNULL(trl.PassIfMatch, 0)
 -- CREATING TEXT RESTRICTION RECORDS FOR VALID AND UNIQUE TEXT RESTRICTION COMBINATION --
 INSERT INTO dbo.SMSChannelNetworkTextRestriction
 ( TextRestrictionTypeId, [Enabled], SMSChannelId, NetworkId, GatewayId, CountryId, PassIfMatch )
 SELECT 
 trl.TextRestrictionTypeId
 ,trl.[Enabled]
 ,trl.SMSChannelId
 ,trl.NetworkId
 ,trl.GatewayId
 ,trl.CountryId
 ,trl.PassIfMatch
 FROM
 @TextRestrictionList trl
 LEFT JOIN
 dbo.SMSChannelNetworkTextRestriction tr
 ON 
 tr.TextRestrictionTypeId = trl.TextRestrictionTypeId
 AND (tr.SMSChannelId = trl.SMSChannelId OR ISNULL(tr.SMSChannelId, trl.SMSChannelId) IS NULL)
 AND (tr.NetworkId = trl.NetworkId OR ISNULL(tr.NetworkId , trl.NetworkId ) IS NULL)
 AND (tr.GatewayId = trl.GatewayId OR ISNULL(tr.GatewayId , trl.GatewayId ) IS NULL)
 AND (tr.CountryId = trl.CountryId OR ISNULL(tr.CountryId , trl.CountryId ) IS NULL)
 LEFT JOIN
 #InconsistentRestrictions ir
 ON 
 ir.TextRestrictionTypeId = trl.TextRestrictionTypeId
 AND (ir.SMSChannelId = trl.SMSChannelId OR ISNULL(ir.SMSChannelId, trl.SMSChannelId) IS NULL)
 AND (ir.NetworkId = trl.NetworkId OR ISNULL(ir.NetworkId , trl.NetworkId ) IS NULL)
 AND (ir.GatewayId = trl.GatewayId OR ISNULL(ir.GatewayId , trl.GatewayId ) IS NULL)
 AND (ir.CountryId = trl.CountryId OR ISNULL(ir.CountryId , trl.CountryId ) IS NULL)
 WHERE 
 ir.TextRestrictionTypeId IS NULL AND tr.Id IS NULL
 GROUP BY
 trl.TextRestrictionTypeId
 ,trl.[Enabled]
 ,trl.SMSChannelId
 ,trl.NetworkId
 ,trl.GatewayId
 ,trl.CountryId
 ,trl.PassIfMatch
 -- INSERTING VALID NEW EXPRESSIONS --
 INSERT INTO dbo.TextRestrictionData
 ( SMSChannelNetworkTextRestrictionId, Expression )
 SELECT 
 tr.Id
 ,trl.Expression
 FROM 
 @TextRestrictionList trl
 LEFT JOIN
 #InconsistentRestrictions ir
 ON 
 ir.TextRestrictionTypeId = trl.TextRestrictionTypeId
 AND (ir.SMSChannelId = trl.SMSChannelId OR ISNULL(ir.SMSChannelId, trl.SMSChannelId) IS NULL)
 AND (ir.NetworkId = trl.NetworkId OR ISNULL(ir.NetworkId , trl.NetworkId ) IS NULL)
 AND (ir.GatewayId = trl.GatewayId OR ISNULL(ir.GatewayId , trl.GatewayId ) IS NULL)
 AND (ir.CountryId = trl.CountryId OR ISNULL(ir.CountryId , trl.CountryId ) IS NULL)
 LEFT JOIN
 dbo.SMSChannelNetworkTextRestriction tr
 ON
 tr.TextRestrictionTypeId = trl.TextRestrictionTypeId
 AND (tr.SMSChannelId = trl.SMSChannelId OR ISNULL(tr.SMSChannelId, trl.SMSChannelId) IS NULL)
 AND (tr.NetworkId = trl.NetworkId OR ISNULL(tr.NetworkId , trl.NetworkId ) IS NULL)
 AND (tr.GatewayId = trl.GatewayId OR ISNULL(tr.GatewayId , trl.GatewayId ) IS NULL)
 AND (tr.CountryId = trl.CountryId OR ISNULL(tr.CountryId , trl.CountryId ) IS NULL)
 WHERE
 ir.TextRestrictionTypeId IS NULL AND tr.Id IS NOT NULL
 COMMIT;
 -- RETURN INFORMATION ABOUT INCONSISTENT RESTRICTION RECORDS --
 SELECT ir.TextRestrictionTypeId
 ,ir.[Enabled]
 ,ir.SMSChannelId
 ,ir.NetworkId
 ,ir.GatewayId
 ,ir.CountryId
 ,ir.Expression
 ,ir.PassIfMatch
 FROM #InconsistentRestrictions ir
END TRY
BEGIN CATCH
 ROLLBACK TRANSACTION;
 THROW;
END CATCH

• rwking: Hello. Would it be possible for you to post your actual execution plan?
• rwking: Other questions: What volume of records are we talking? A few thousand, tens of thousands, millions, etc? Do you have an indexing strategy on your permanent tables? If so, what do your indexes look like? Even good code can run poorly if the indexing strategy stinks. Still looking at your code...
• kerem: You can use sqlfiddle.com to share schema and queries.

Answer 1

Note that the clauses like

ISNULL(tr.SMSChannelId, trl.SMSChannelId) IS NULL

and

ISNULL(tr.PassIfMatch, 0) != ISNULL(trl.PassIfMatch, 0)

are non-SARG-able, and will thus force a table scan. In fact It is possible that your first large JOIN will be run as a single pass through a cross join of the two tables.

The first thing you can do is to recode clauses like (1) above as

(tr.SMSChannelId IS NULL OR trl.SMSChannelId IS NULL)

Ugly, but necessary.

However on those tables over which you have schema-control, such as @TextRestrictionList and #InconsistentRestrictions, you can do better by making some of the nullable columns non-nullable.

NULL strings can often be converted to empty strings without any semantic change to queries.

NULL numbers can often be converted to one of 0, -1, or MinValue without any semantic change to queries.

Null Dates can usually be converted to one of '001-01-01' or '9999-12-31' without any semantic change to queries.

By performing these changes where possible you will both simplify your query syntax and give the engine optimizer better capability to use indices to optimize your query.