2
\$\begingroup\$

Without changing the semantics and performance, how can I make this where clause more readable?

where
(@Category = 'all' or
    (@Category = 'omitted' and Category is null) or
    (@Category = Category and (
        @SubCategory = 'all' or
        (@SubCategory is null and SubCategory is null) or
        (@SubCategory = SubCategory and (
            @SubSubCategory = 'all' or
            (@SubSubCategory is null and SubSubCategory is null) or
            @SubSubCategory = SubSubCategory
        ))
    ))
)

At a high level, this clause represents a user query where the user has optionally specified a category, subcategory, and sub-subcategory on a table. For compatibility reasons, I cannot change any semantics (e.g., I cannot improve consistency by using @Category is null instead of @Category = 'omitted')

\$\endgroup\$
3
\$\begingroup\$

Add comments, they're the usual way of making complicated source code easier to understand.

There's not a lot to add in your case because the meaning of the code is simple even if the implementation isn't easy to take in at a glance, but adding a few short comments may make it much more readable, especially for someone who's reading through the whole thing trying to understand the flow of logic. I would just do this:

where
    -- optional category
    (@Category = 'all' or
    (@Category = 'omitted' and Category is null) or
    (@Category = Category and (
        -- optional subcategory
        @SubCategory = 'all' or
        (@SubCategory is null and SubCategory is null) or
        (@SubCategory = SubCategory and (
            -- optional subsubcategory
            @SubSubCategory = 'all' or
            (@SubSubCategory is null and SubSubCategory is null) or
            @SubSubCategory = SubSubCategory
        ))
    ))
)

Now I can quickly see the purpose of the code, and even if the detailed logic is awkward to follow it may not matter at all because I may not need to change it, just understand what it's doing. If I do need to modify it, I'll have no choice but to work through it in detail anyway, in which case the comments will help to point me in the right direction.

You didn't mention if you're using SQL Server or Sybase, but if it's SQL Server and if you haven't read it already, this article is well worth reading. I wouldn't be too dogmatic about avoiding dynamic SQL, it certainly has its issues but it has its uses too.

\$\endgroup\$
0
0
\$\begingroup\$

This kind of condition may actually hurt performance quite a lot, because query optimizer most likely will be confused by complex filtering condition and would have to do a table scan rather than index seek (assuming that you have index over Category and SubCategory). In order to improve querying performance you should simplify WHERE clause.

Best solution would be to utilize client-side ORM capabilities to generate dynamic filtering based on available data. If that isn't possible, then I would go with one of the following approaches (I would prefer Multiple queries approach in case when select statement is simple):

  • Dynamic sql:

    declare @sql nvarchar(max);
    set @sql = 'select [field_list] from table_name ' + case 
        when @Category = 'all' then ''
        when @Category = 'omitted' then 'where Category is null'
        else 'where Category = @Category' + case
            when @SubCategory = 'all' then ''
            when @SubCategory is null then ' and SubCategory is null'
            else ' and SubCategory = @SubCategory' + case
                when @SubSubCategory = 'all' then ''
                when @SubSubCategory is null then ' and SubSubCategory is null'
                else ' and SubSubCategory = @SubSubCategory'
            end
        end
    end
    
    exec sp_executesql @stmt = @sql,
        @params = N'@Category varchar(100), @SubCategory varchar(100), @SubSubCategory varchar(100)',
        @Category = @Category,
        @SubCategory = @SubCategory,
        @SubSubCategory = @SubSubCategory;
    
  • Multiple queries approach:

    if (@Category = 'all')
        select [field_list]
        from table_name
    else if (@Category = 'omitted')
        select [field_list]
        from table_name
        where Category is null
    else if (@SubCategory = 'all')
        select [field_list]
        from table_name
        where Category = @Category
    else if (@SubCategory is null)
        select [field_list]
        from table_name
        where Category = @Category and SubCategory is null
    else if (@SubSubCategory = 'all')
        select [field_list]
        from table_name
        where Category = @Category and SubCategory = @SubCategory
    else if (@SubSubCategory is null)
        select [field_list]
        from table_name
        where Category = @Category and SubCategory = @SubCategory and SubSubCategory is null
    else
        select [field_list]
        from table_name
        where Category = @Category and SubCategory = @SubCategory and SubSubCategory = @SubSubCategory
    
\$\endgroup\$
5
  • \$\begingroup\$ I refuse to replace working code with dynamic SQL. I would put up with your second solution, but considering the complexity of my select clause, I dislike this level of code repetition. The best of both worlds might be to make use of your second solution in a UDF, but not until MS fixes Scalar UDF Performance \$\endgroup\$ – Brian Apr 10 '13 at 15:15
  • \$\begingroup\$ Does your code run over indices with that kind of WHERE clause? I doubt it. So even if it does work on small amount of data, dynamic SQL will beat it on larger data (assuming that you have corresponding index). Of course it's still kind of hack, because this type of conditions should be evaluated on client side using ORMs \$\endgroup\$ – almaz Apr 10 '13 at 15:24
  • \$\begingroup\$ Actually, it does have an index on those columns. Regardless, this is moot; my team has a policy against using dynamic SQL unless absolutely necessary. Personally, I consider your dynamic SQL approach to be just as difficult to read. \$\endgroup\$ – Brian Apr 10 '13 at 15:29
  • \$\begingroup\$ I asked about whether the query runs over index (does the index seek operation, not whether you have index. Both my options were attempts to improve performance rather than readability. \$\endgroup\$ – almaz Apr 10 '13 at 21:17
  • \$\begingroup\$ OK, I'll be more explicit, it does run over the index. Anyhow, please note my original question, "Without changing the semantics and performance, how can I make this where clause more readable?" Performance is fine. Readability is my only concern. \$\endgroup\$ – Brian Apr 10 '13 at 23:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.