5

I come across an article describing different situation in which the SQL code is probably not correct. However, there is one point which is surprising to me. They claim

it is wise to explicitly handle NULLs in nullable columns, by using COALESCE to provide a default value

ISNULL is mentioned as well. They also reference this MSDN web page giving an example with ISNULL. The basic idea here is that it is better to use

SELECT COUNT(*) FROM [dbo].[Table1] WHERE ISNULL([c2],0) > 2;

then

SELECT COUNT(*) FROM [dbo].[Table1] WHERE [c2] > 2;

However, the first variant will not be SARG, whereas, the result is not influenced by ISNULL at all. I understand the need to handle NULL using ISNULL or COALESCE in the output, however, I always try to use IS NULL or IS NOT NULL to handle NULL in the predicate. Do I miss something? What is the point of the MSDN issue?

EDIT: in order to react on the discussion and mainly on this post I have prepared a simple test

 IF OBJECT_ID('dbo.LogTable', 'U') IS NOT NULL  DROP TABLE dbo.LogTable

 SELECT TOP 100000 DATEADD(day, ( ABS(CHECKSUM(NEWID())) % 65530 ), 0) datesent ,
      CASE WHEN ( ABS(CHECKSUM(NEWID())) % 100 ) = 1 THEN NULL ELSE ( ABS(CHECKSUM(NEWID())) % 1000 ) END ivalue
 INTO [LogTable]
 FROM    sys.sysobjects
 CROSS JOIN sys.all_columns

 CREATE INDEX ix_logtable_ivalue ON LogTable(ivalue asc) INCLUDE(datesent);

 -- Q1
 select * from logtable where isnull(ivalue, 0) > 998

 -- Q2
 select * from logtable where ivalue > 998

However, the ivalue in Q1 is not SARG. Is there any catch? How should I make the attribute SARG for this particular data and query?

Radim Bača
  • 10,646
  • 1
  • 19
  • 33
  • 4
    You are correct. Don't put in unnecessary `NULL` checks, because that can impede the use of indexes. I strongly recommend `IS NULL`/`IS NOT NULL`. Those are the ANSI standard constructs. – Gordon Linoff Oct 04 '17 at 11:01
  • 9
    The fact that that example is coming from something that purports to detect bad database code for improvement is very worrisome. You *do* need to think about what could happen when you compare values that can be `NULL`, but reflexively putting `ISNULL` everywhere is downright wrong. – Jeroen Mostert Oct 04 '17 at 11:10
  • `What is the point of the MSDN issue`? Dunno :-) The remark from the [original source](https://www.red-gate.com/simple-talk/sql/t-sql-programming/sql-code-smells/#not-handling-null-values-in-nullable-columns) makes sense whilst calculating things, but the given example from [MSDN](https://msdn.microsoft.com/en-us/library/dd172133(v=vs.100).aspx) is completely mistaken. – Marcus Vinicius Pompeu Oct 05 '17 at 05:35

1 Answers1

6

The isnull check in the example you provided is pointless. null > 2 returns null, which is not "true", and thus these rows will be excluded from the query anyway. To boot, using isnull in that fashion will prohibit the optimizer from using the index on c2 if you have one.

In short - this sounds like poor advice.

Mureinik
  • 297,002
  • 52
  • 306
  • 350
  • 1
    "null > 2 returns null" - nope, this comparison returns unknown. A bit pedantic but nulls are confusing. The point is still correct - there is no reason to check for null. – SMor Oct 04 '17 at 13:55
  • 1
    @SMor: very pedantic, since the boolean data type exists only virtually in T-SQL. If it existed in actuality (I wish, it would be quite useful), it's almost certain that the `UNKNOWN` outcome would be represented by `NULL`. I'm not sure how `NULL` would be "more confusing" than `UNKNOWN`, though, since that's what the thing is there for in the first place. – Jeroen Mostert Oct 04 '17 at 15:17
  • `using isnull in that fashion will prohibit the optimizer from using the index on c2 if you have one` is not quite right as may still be issued an `index scan`, as [Pankaj Manek brightly shows us here](http://www.sqlservercentral.com/blogs/sqlyse-with-pankaj-manek/2015/08/20/isnull-around-the-predicate-and-sargability/). The statement holds true for `index seek`. – Marcus Vinicius Pompeu Oct 05 '17 at 05:44