January 10, 2011 at 11:32 am
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?
January 10, 2011 at 12:26 pm
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:
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
January 10, 2011 at 12:28 pm
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
January 10, 2011 at 12:58 pm
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.
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
January 10, 2011 at 1:04 pm
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).
January 10, 2011 at 1:07 pm
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.
January 10, 2011 at 1:17 pm
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.
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
January 10, 2011 at 6:34 pm
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
Change is inevitable... Change for the better is not.
January 11, 2011 at 10:41 am
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
January 11, 2011 at 10:47 am
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
January 11, 2011 at 11:27 am
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
😀
January 11, 2011 at 12:05 pm
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.
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
January 13, 2011 at 6:37 am
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
January 14, 2011 at 8:17 am
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
January 14, 2011 at 6:18 pm
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
Change is inevitable... Change for the better is not.
Viewing 15 posts - 1 through 14 (of 14 total)
You must be logged in to reply to this topic. Login to reply