string building

  • I'm dynamically building a WHERE statement based on parameters passed to a proc. Is there a better way (maybe a CASE statement) to build the string based on what parameters are passed rather then testing every variable to see if it's entered or empty.

    CREATE PROCEDURE App_Log

    @Severity Int

    , @UserName VARCHAR(24)

    , @OrderId INT

    , @ServicId INT

    , @RequestId VARCHAR(64)

    , @EventId INT

    , @KeyId UNIQUEIDENTIFIER

    , @Id BIGINT

    , @Machine VARCHAR(48)

    , @Location VARCHAR(256)

    , @Message VARCHAR(256)

    AS

    BEGIN

    DECLARE @sql VARCHAR(1000)

    SET @sql = 'SELECT

    [Id]

    ,[TimeStamp]

    ,[Severity]

    ,[Category]

    ,[EventId]

    ,[UserName]

    ,[KeyId]

    ,[Param1]

    ,[Param2]

    ,[Param3]

    ,[Message]

    ,[Location]

    ,[MachineName]

    ,[ApplicationName]

    ,[AppDomainName]

    ,[Identity]

    ,[DetailMessageSize]

    FROM [Logger2]

    WITH(NOLOCK) WHERE '

    IF @Severity <> ''

    BEGIN

    SET @sql = @sql + '[Severity] = ' + @Severity

    END

    IF @UserName <> ''

    BEGIN

    IF @Severity <> ''

    BEGIN

    SET @sql = @sql + 'AND [UserName] = ' + @UserName

    END

    ELSE

    BEGIN

    SET @sql = @sql + '[UserName] = ' + @UserName

    END

    END

  • I always avoid using dynamic SQL. Your WHERE clause could look like this instead:

    WHERE (Username = @Username OR @Username = '')

    Basically it will use the username if it's supplied or remove it from the where clause if it's not.

    Greg

  • I agree with Greg. Try to avoid dynamic sql.

  • I'd like to avoid using dynamic sql, so I'm open to any suggestions. There are 12 potential parameters in all.

    I tried Greg's suggestion, but in my case, if the variable is not passed, then I need ALL values (vs empty).

    SELECT

    [Id]

    ,[TimeStamp]

    ,[Severity]

    ,[Category]

    ,[EventId]

    ,[UserName]

    ,[KeyId]

    ,[Param1]

    ,[Param2]

    ,[Param3]

    ,[Message]

    ,[Location]

    ,[MachineName]

    ,[ApplicationName]

    ,[AppDomainName]

    ,[Identity]

    ,[DetailMessageSize]

    FROM [Logger2]

    WITH(NOLOCK)

    WHERE ([Severity] = @Severity OR [Severity] <> '')

    AND ([UserName] = @UserName OR [UserName] <>'')

    AND ([Param1] = @OrderId OR [Param1] <>'')

    AND ([Param2] = @ServiceId OR [Param2] <> '')

    AND ([Param3] = @RequestId OR [Param3] <> '')

    AND ([EventId] = @EventId OR [EventId] <> '')

    --AND ([KeyId] = @KeyId OR [KeyId] <> '')

    AND ([Id] = @Id OR [Id] <> '')

    AND ([MachineName] = @Machine OR [MachineName] <> '')

    AND ([Location] = @Location OR [Location] <> '')

    AND ([Message] = @Message OR [Message] <> '')

  • You have ([Severity] = @Severity OR [Severity] <> '')

    It should be ([Severity] = @Severity OR @Severity = '')

    If the value that is passed in is an empty string, it will not evaluate it and return all.

    Greg

  • Hi

    Unless you dont have any way to go thenonly use Dynamic SQL, otherwise follow Greg's suggestion. This will improve the query performance.

    Thanks -- Vijaya Kadiyala

    http://dotnetvj.blogspot.com

  • AAAhhhhh I seeee said the blind man.....thanks Greg!!!

  • [font="Verdana"]There are some issues with performance that dynamic SQL can solve. I'd try the solution without dynamic SQL first, and if it becomes a performance issue, revisit.

    Another option is that you can actually use an if statement within your stored procedure to select which statement to run, and code the statements accordingly. That means you write a lot more code, but there's no dynamic SQL and (I believe, was true in SQL 2000, someone else may be able to tell you whether it's still the case) you avoid some of the parameter sniffing issues.

    So your code would look something like:

    if (@p1 is not null and @p2 is not null and @p3 is not null)

    -- do select with all three parameters

    else if (@p1 is not null and @p2 is not null)

    -- do select with @p1 and @p2

    etc

    [/font]

  • I don't think you would want to go with if statements. There are 12 different potential variables in this case, which equals a whole lot of possibilities! I've seen this handled as one statement two ways. Either like I stated above: (Column1 = @Column1 OR @Column1 = {some default value}) or through a case statement like this: Column1 = CASE WHEN @Column1 = {some default value} THEN Column1 ELSE @Column1 END. I'm not sure which is better performance-wise, but both accomplish the same task.

    Greg

  • [font="Verdana"]Agreed. I'm not fond of using if statements this way. However, I remember that in SQL Server 7 and 2000, it was the best performing option. Then dynamic SQL. Then the single statement with ORs for null parameters.

    It's a lot of code, and I always think it's better to aim for more maintainable code than anything else. But I figured it was worth listing as an option.

    [/font]

  • If you do run into a performance issue using the structure:

    WHERE (column = @parm OR @parm IS NULL)

    You can either move directly to Dynamic SQL, or you can try using the recompile option on the procedure. Only use the recompile option if the procedure is not something that will be executed a lot.

    Jeffrey Williams
    “We are all faced with a series of great opportunities brilliantly disguised as impossible situations.”

    ― Charles R. Swindoll

    How to post questions to get better answers faster
    Managing Transaction Logs

  • much thanks for the comments..we are using if's and they seem to preform okay...

    Why do I have a feeling the case statments would have worked better..or..some other form of case..? Bruce your code seems good but we have one field guid that s new to us..had issues with the convert and ran into isssues with guid (no reason to have this but what can u do?).

    WHERE (column = @parm OR @parm IS NULL)

    we tried something like above..but it becomes a 'tough to keep up with string' when they can type in ad hoc statements..like..sometimes with qoutes somtimes without.

    where datetime = '02/20/2009'

    Gail sent a link..ths suggestion there worked pretty good.

  • [font="Verdana"]Ah. The old "don't assume user input is valid".

    If they are typing dates, sometimes with quotes, sometimes without, then you need to ensure that any date passed to your stored procedure is a SQL datetime type. That way by the time it hits your stored procedure, you know it's a valid date. Any type checking should be pushed back into the application.

    The stronger you make your types, the less problems you will encounter.

    Feel free to post up any subsequent issues you are running in to!

    [/font]

  • Hi krypto,

    instead of using Dynamic SQL and othe advices Follow Please use coalesce Like Below .. Do u still have problem let me know

    SELECT

    Id

    ,TimeStamp

    ,Severity

    ,Category

    ,EventId

    ,UserName

    ,KeyId

    ,Param1

    ,Param2

    ,Param3

    ,Message

    ,Location

    ,MachineName

    ,ApplicationName

    ,AppDomainName

    ,Identity

    ,DetailMessageSize

    FROM Logger2 WITH(NOLOCK)

    WHERE Isnull(Severity,'') = Coalesce(@Severity,Severity,'')

    AND Isnull(UserName,'') = Coalesce(@UserName,UserName,'')

    AND Isnull(Param1,'') = Coalesce(@OrderId,Param1,'')

    AND Isnull(Param2m'') = Coalesce(@ServiceId,Param2,'')

    AND Isnull(Param3,'') = Coalesce(@RequestId,Param3,'')

    AND Isnull(EventId,0) = Coalesce(@EventId,EventId,0)

    AND Isnull(Id,0) = Coalesce(@Id,Id,0)

    AND Isnull(MachineName,'') = Coalesce(@Machine,MachineName,'')

    AND Isnull(Location,'') = Coalesce(@Location,Location,'')

    AND Isnull(Message,'') = Coalesce(@Message,Message,'')

  • I had this problem at an earlier job and it was a dog - arf! Making the WHERE select into a temp file helped a bit. If you have to go the dymanic SQL route, I am told that sp_executesql is a trifle faster.

Viewing 15 posts - 1 through 15 (of 15 total)

You must be logged in to reply to this topic. Login to reply