1

The below stored proc works fine except for the fact that when I uncomment the second part of the date check in the 'where' clause it blows up on a date conversion even if the passed in keyword is null or '111'.

I'm open to any suggestions on how to do this dynamic where clause differently.

I appreciate any help.

 ALTER PROCEDURE [SurveyEngine].[GetPageOf_CommentsOverviewRowModel]
        @sortColumn varchar(50),
        @isASC bit,
        @keyword varchar(50)
    AS
    BEGIN

        declare @keywordType varchar(4)
        set @keywordType = null

        if ISDATE(@keyword) = 1
            set @keywordType = 'date'
        else if ISNUMERIC(@keyword) = 1
            set @keywordType = 'int'

        select      c.CommentBatch BatchID, c.CreatedDate DateReturned, COUNT(c.CommentID) TotalComments
        from        SurveyEngine.Comment c 
        where       (@keywordType is null)
        or          (@keywordType = 'date') --and c.CreatedDate = @keyword)
        or          (@keywordType = 'int' and (CONVERT(varchar(10), c.CommentBatch) like  @keyword+'%'))

        group by    c.CommentBatch, c.CreatedDate
        order by    case when @sortColumn = 'BatchID' and @isASC = 0 then c.CommentBatch end desc,
                    case when @sortColumn = 'BatchID' and @isASC = 1 then c.CommentBatch end,
                    case when @sortColumn = 'DateReturned' and @isASC = 0 then c.CreatedDate end desc,
                    case when @sortColumn = 'DateReturned' and @isASC = 1 then c.CreatedDate end,
                    case when @sortColumn = 'TotalComments' and @isASC = 0 then COUNT(c.CommentID) end desc,
                    case when @sortColumn = 'TotalComments' and @isASC = 1 then COUNT(c.CommentID) end
    END
Keith Myers
  • 1,379
  • 2
  • 15
  • 29
  • May be of interest: http://stackoverflow.com/questions/206484/sql-switch-case-in-where-clause –  May 10 '12 at 22:34
  • I checked that one before posting but my real issue is why the 'and' operator does not prevent the second statement from being evaluated when @keywordType is not 'date'. There must be something incredibly obvious but I am not seeing it. – Keith Myers May 10 '12 at 23:08
  • Assuming c.CraetedDate is a date then maybe or (@keywordType = 'date' and c.CreatedDate = Convert(DateTime,@keyword)) ? – Tony Hopkinson May 10 '12 at 23:14
  • I should add that if you feed it a correctly formatted date it works. So, if keyword = '1/1/1999' all is well. If keyword = 'zzz' it will blow up on a date conversion. – Keith Myers May 10 '12 at 23:19
  • hmm Convert(VarChar(10),c.CreatedDate) = Keyword would remove the implicit conversion but it would mean KeyWord would have to have specific format, when it contained a date. – Tony Hopkinson May 10 '12 at 23:32

2 Answers2

2

Based on this guy's blog: http://blogs.msdn.com/b/bartd/archive/2011/03/03/don-t-depend-on-expression-short-circuiting-in-t-sql-not-even-with-case.aspx it looks like you can't guarantee the order of operations in the where clause, even though short circuiting is supported. The execution plan may choose to evaluate the second statement first.

He recommends using a case structure instead (like pst mentioned before) as it is "more" gauranteed. But I don't think I can rewrite your where clause as a case because you're using three different operators (is null, =, and LIKE).

Bill Moniz
  • 311
  • 1
  • 3
2

EDIT Sorry, brain cloud. Things need to be initialized differently.

Change the setup to:

    declare @keywordType varchar(4)
    declare @TargetDate as DateTime = NULL

    set @keywordType = null 

    if ISDATE(@keyword) = 1
        begin
        set @keywordType = 'date'
        set @TargetDate = Cast( @keyword as DateTime )
        end
    else if ISNUMERIC(@keyword) = 1 
        set @keywordType = 'int' 

Then change:

and c.CreatedDate = @keyword

to:

and c.CreatedDate = Coalesce( @TargetDate, c.CreatedDate )

That will result in a NOP if you are not searching by date.

HABO
  • 15,314
  • 5
  • 39
  • 57
  • This worked and allowed me to move on with my project at work. Also nice to learn about the Coalesce, which looks quite useful. – Keith Myers May 11 '12 at 12:35