February 11, 2009 at 8:46 am
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
February 11, 2009 at 8:57 am
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
February 11, 2009 at 9:28 am
I agree with Greg. Try to avoid dynamic sql.
February 11, 2009 at 9:33 am
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] <> '')
February 11, 2009 at 9:40 am
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
February 11, 2009 at 9:43 am
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
February 11, 2009 at 9:45 am
AAAhhhhh I seeee said the blind man.....thanks Greg!!!
February 12, 2009 at 12:19 pm
[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]
February 12, 2009 at 12:33 pm
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
February 12, 2009 at 1:16 pm
[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]
February 12, 2009 at 1:28 pm
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
February 12, 2009 at 9:54 pm
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.
February 15, 2009 at 12:21 pm
[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]
February 16, 2009 at 11:26 pm
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,'')
February 19, 2009 at 2:49 pm
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