I'm reviewing some code I've inherited, and I found a line like this:
And isnull(IH.CustomerItemNumber, '') <> ''
Which one of my predecessors seems to have used in a ton of where clause or join clauses. It looks to me like this is an unnecessary calling of a function, and therefor a performance hog, because NULL will never equal the empty string '', right?
Specifically I took this out of a join clause in a particular query and performance increased dramatically (from 46-49 secs to between 1-3).
Replaced it with AND IH.CustomerItemNumber <> ''
Is my assessment correct here? This is redundant and slow and can be removed? In what situation might this code be beneficial?
EDIT: So, can NULL ever equal the empty string?
The reason that the code is there may be because of the history of the application. Perhaps at some point in time, NULLs were allowed in the field. Then, these were replaced with empty strings.
The reason the code is inefficient is because of the optimization of the join. ISNULL() and its ANSI standard equivalent COALESCE() generally add negligible overhead to the processing of a query. (It does seem that in some versions of SQl Server, COALESCE() evaluates the first argument twice, which is a problem if it is a subquery.)
My guess is that the field has an index on it. SQL Server knows to use the index for the join when the field is used alone. It is not smart enough to use the index when included in a function call. It is the join optimziation that is slowing down the query, not the overhead of the function call.
Personally, I would prefer the form with the explicit NULL check, if the performance is the same:
IH.CustomerItemNumber <> '' and IH.CustomerItemNumber is not null
Being explicit about NULL processing can only help you maintain the code in the future.