Proc Performance

  • I have a proc that has this

    Create proc myproc

    @param1 int = null,

    @param2 varchar = null,

    @param3 int = null,

    @param4 varchar = null,

    AS

    BEGIN

    @DECLARE @sql varchar(max) =

    'SELECT * FROM MYTABLE

    WHERE 1=1'

    IF @param2 IS NOT NULL

    SET @sql = @sql + ' AND param = ''' + @param2 + ''''

    etc

    I dont think where 1 = 1 is very clean code just to be able to add/remove params easier based on their availability. I want to change it to something like:

    @DECLARE @WHERE VARCHAR(MAX) = ' WHERE'

    @DECLARE @sql varchar(max) =

    'SELECT * FROM MYTABLE'

    IF @param2 IS NOT NULL

    BEGIN

    IF LEN(@where) > 7

    SET @WHERE = @WHERE + ' AND param = ''' + @param2 + ''''

    ELSE

    SET @WHERE = @WHERE + ' param = ''' + @param2 + ''''

    END

    I was wondering if the second way would affect performance negatively?

  • It looks like what you're trying to do is a "catch-all" proc, of the sort usually built for complex search pages. If so, take a look at the entry on this site on that kind of proc:

    http://sqlinthewild.co.za/

    Gail wrote a good bit about it, and it will definitely help.

    As usualy, Joe is right about the technical aspects he wrote, and renders his own post completely useless by its arrogant style. So, ignore it, or read it with the understanding that he means well, but chooses to harm his own cause by his writing style.

    - Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
    Property of The Thread

    "Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon

  • There are several problems with your procedure and the "WHERE (1 = 1)" clause is not really one of them.

    You should always declare the length of any varchar, nvarchar, char or nchar parameters.

    If possible, your query should specify the list of columns to retrieve rather than use 'SELECT * FROM'

    Your procedure is vulnerable to SQL injection attacks that could compromise or corrupt your data, or alternatively the procedure may generate invalid SQL if either of the varchar parameters @param2 or @param4 includes an apostophe character.

    You should construct your dynamic sql to use parameters and use sp_executesql. This will allow SQL Server to use a cached query plan, thereby improving query performance, and will help to protect you against SQL injection attacks.

    Although not essential, the "WHERE (1 = 1)" constrct remains useful in that it can significantly reduce the amount of conditional code required in the stored procedure if all the input parameters are optional. The Query optimizer should ignore the "(1 = 1)" filter, so it will not affect performance either way.

    I would rewite the procedure something like this:

    CREATE PROC MyProc

    @param1 int = NULL,

    @param2 varchar(50) = NULL,

    @param3 int = NULL,

    @param4 varchar(50) = NULL

    AS

    DECLARE @sql nvarchar(4000)

    DECLARE @plst nvarchar(4000)

    SET @sql = N'SELECT col1, col2, col3, col4 FROM dbo.MyTable WHERE (1=1)'

    SET @plst = N'@p1 int, @p2 varchar(50), @p3 int, @p4 varchar(50)'

    IF (@param1 IS NOT NULL)

    SET @sql = @sql + N' AND (param1 = @p1)'

    IF (@param2 IS NOT NULL)

    SET @sql = @sql + N' AND (param2 = @p2)'

    IF (@param3 IS NOT NULL)

    SET @sql = @sql + N' AND (param3 = @p3)'

    IF (@param4 IS NOT NULL)

    SET @sql = @sql + N' AND (param4 = @p4)'

    --PRINT @sql

    EXEC sp_executesql @sql, @plst, @param1, @param2, @param3, @param4

    GO

  • To answer your question directly:

    No, removing the 1=1 component in the where clause will not affect performance. As a matter of fact, 1=1 never does anything in the optimizer, it's just to reduce the amount of CASE code in your query to ease maintenance from having to list every possible where component twice.

    There was a recent article on short-circuiting that will explain what the optimizer will due when faced with that equivalency. It's just there to make things easier.


    - Craig Farrell

    Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.

    For better assistance in answering your questions[/url] | Forum Netiquette
    For index/tuning help, follow these directions.[/url] |Tally Tables[/url]

    Twitter: @AnyWayDBA

  • This was an oversimplified version of my proc. I wrongfully assumed that if I ask a question about the where clause, that people would answer the question about the where clause.

    I do not have a * operator in my actual proc. I do have the length of each data type specified. I AM using sp_executesql. I just wasn't going to type all of that in because my question was about the where clause. But now that you guys have gotten the jist of my question. I can perhaps ask it more clearly

    Will the excess conditional statements will affect performance negatively? I already know that query optimizer will ignore the where 1 = 1 part.

    Thank you for the 2 of you that actually tried answering my question. Don't worry I ignored the guy trying to sell his book (interesting way of advertising by insulting the people you want to sell it to).

  • also it is bad proctice to default ur input params to null. If its a varchar it's not so bad however dates. Not so good.

  • siamak.s16 (1/10/2011)


    This was an oversimplified version of my proc. I wrongfully assumed that if I ask a question about the where clause, that people would answer the question about the where clause.

    Joe's kinda rabid... he keeps missing the concept that people post SAMPLE code instead of the entire database DDL so he can have more ready made examples for his books... at least that's why I assume he's being so dense usually.

    Will the excess conditional statements will affect performance negatively? I already know that query optimizer will ignore the where 1 = 1 part.

    Thank you for the 2 of you that actually tried answering my question.

    You're welcome. No, it won't, not really. In the 1 millisecond range if anything, nothing to be concerned about. It's more a maintenace issue then an optimizer one for that portion.


    - Craig Farrell

    Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.

    For better assistance in answering your questions[/url] | Forum Netiquette
    For index/tuning help, follow these directions.[/url] |Tally Tables[/url]

    Twitter: @AnyWayDBA

  • BaldingLoopMan (1/10/2011)


    also it is bad proctice to default ur input params to null. If its a varchar it's not so bad however dates. Not so good.

    I have to ask... why? What if you have criteria that doesn't include one of the parameters?

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

  • Jeff Moden (1/10/2011)


    BaldingLoopMan (1/10/2011)


    also it is bad proctice to default ur input params to null. If its a varchar it's not so bad however dates. Not so good.

    I have to ask... why? What if you have criteria that doesn't include one of the parameters?

    I'm wondering the same thing.

    - Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
    Property of The Thread

    "Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon

  • siamak.s16 (1/10/2011)


    This was an oversimplified version of my proc. I wrongfully assumed that if I ask a question about the where clause, that people would answer the question about the where clause.

    I do not have a * operator in my actual proc. I do have the length of each data type specified. I AM using sp_executesql. I just wasn't going to type all of that in because my question was about the where clause. But now that you guys have gotten the jist of my question. I can perhaps ask it more clearly

    Will the excess conditional statements will affect performance negatively? I already know that query optimizer will ignore the where 1 = 1 part.

    Thank you for the 2 of you that actually tried answering my question. Don't worry I ignored the guy trying to sell his book (interesting way of advertising by insulting the people you want to sell it to).

    Did you read the article on Gail's blog about this kind of query?

    The "1=1" part just makes writing the dynamic SQL build easier. I'd leave it in there, since it's not hurting anything.

    - Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
    Property of The Thread

    "Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon

  • I am not understanding, why you are using dynamic sql string?

    If My understanding is correct, you need to set the arguments in the where clause based on the parameter value. Please see below code.

    CREATE PROC MyProc

    @param1 int = NULL,

    @param2 varchar(50) = NULL,

    @param3 int = NULL,

    @param4 varchar(50) = NULL

    AS

    SELECT col1, col2

    FROM mytable

    WHERE (ISNull(@param1,0) = 0 Or parm = @param1)

    OR

    (ISNull(@param2,'') = '' Or parm = @param2)

    OR

    (ISNull(@param3,0') = 0 Or parm = @param3)

    OR

    (ISNull(@param4,'') = '' Or parm = @param4)[/size]

    Thanks.

    Reji PR,
    Bangalore
    😀

  • Reji PR (1/11/2011)


    I am not understanding, why you are using dynamic sql string?

    If My understanding is correct, you need to set the arguments in the where clause based on the parameter value. Please see below code.

    The dynamically built 'catchall' where clause is much more efficient. The one you listed tends to be easier to maintain to those not used to working with them, but when you're dealing with rediculous volumes of records and you're trying to avoid parameter sniffing problems on top of other issues, the dynamic SQL is the way to go.

    Your way does work, but it's one query to rule them all... which is not as effective.


    - Craig Farrell

    Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.

    For better assistance in answering your questions[/url] | Forum Netiquette
    For index/tuning help, follow these directions.[/url] |Tally Tables[/url]

    Twitter: @AnyWayDBA

  • Reji PR (1/11/2011)


    I am not understanding, why you are using dynamic sql string?

    If My understanding is correct, you need to set the arguments in the where clause based on the parameter value. Please see below code.

    CREATE PROC MyProc

    @param1 int = NULL,

    @param2 varchar(50) = NULL,

    @param3 int = NULL,

    @param4 varchar(50) = NULL

    AS

    SELECT col1, col2

    FROM mytable

    WHERE (ISNull(@param1,0) = 0 Or parm = @param1)

    OR

    (ISNull(@param2,'') = '' Or parm = @param2)

    OR

    (ISNull(@param3,0') = 0 Or parm = @param3)

    OR

    (ISNull(@param4,'') = '' Or parm = @param4)[/size]

    That works, but it will almost guarantee that you end up doing table/index scans, instead of seeks, because none of the Where clause elements are SARGable. That means (usually) very poor performance. On a high-activity system, the dynamic version will work faster, because it can use index seeks (assuming the indexes are set up correctly).

    - Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
    Property of The Thread

    "Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon

  • Dynamic SQL is by far the best way to handle this type of scenario I think. I have gotten 5 orders of magnitude performance improvement by using it for conditional searches instead of the classic IS NULL OR construct. For simple cases Gail's blog posts solution can be easy to manage and efficient also.

    Note that cache bloat can be an issue, but the OPTIMIZE FOR AD HOC WORKLOADS database setting can mitigate this nicely.

    Oh, and please ignore Joe Celko completely.

    Best,
    Kevin G. Boles
    SQL Server Consultant
    SQL MVP 2007-2012
    TheSQLGuru on googles mail service

  • TheSQLGuru (1/14/2011)


    I have gotten 5 orders of magnitude performance improvement by using...

    I absolutely agree that properly written dynamic SQL can beat the tar out of most "catch all" code as you say but... Seriously? 100,000 times faster?

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

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

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