Query: simplify comparisons which are result of null semantics in predicates

[ralmsdeveloper]

I would like to know if there is any particular reason to produce this SQL statement, or it is something that happened when the Query Pipeline was rewritten.

cc/ @smitpatel @maumar

dbug: Microsoft.EntityFrameworkCore.Database.Command[20100]
      Executing DbCommand [Parameters=[], CommandType='Text', CommandTimeout='30']
      SELECT [e].[Id], [e].[DateOfEvent], [e].[Description], [e].[Key], [e].[Place], [e].[Price]
      FROM [Events] AS [e]
      WHERE ([e].[Description] = N'VSSUMMIT') AND [e].[Description] IS NOT NULL

Repro

using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using MySample.Domain;
using System;
using System.Linq;

namespace QueryEF3
{
    public class Program
    {
        private static void Main()
        { 
            using (var db = new MySampleContext())
            {
                var @events = db
                    .Events
                    .Where(p => p.Description == "VSSUMMIT")
                    .ToArray();
            }
        } 
    }

    public class MySampleContext : DbContext
    {
        private static readonly ILoggerFactory _loggerFactory =
            LoggerFactory.Create(p => p
                .AddConsole()
                .AddFilter((l) => l == LogLevel.Debug));

        public DbSet<Event> Events { get; set; } 

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder
             .UseSqlServer(@"Server=.\RALMS;Database=efcore3;Trusted_Connection=True;")
             .UseLoggerFactory(_loggerFactory);
        }
    }

    public class Event  
    {
        public Event()
        {
            Key = Guid.NewGuid();
        }

        public int Id { get; set; }
        public Guid Key { get; set; }
        public string Description { get; set; }
        public string Place { get; set; }
        public DateTime DateOfEvent { get; set; }
        public decimal Price { get; set; } 
    }
}

[smitpatel]

What is the issue here? 😕

[ralmsdeveloper]

@smitpatel

This is trivial, the first instruction overrides the second.
AND [e].[Description] IS NOT NULL

[smitpatel]

Description is nullable column.

[ralmsdeveloper]

Yes, it is, but I believe we should approach it differently, if you were to write this manually, it certainly would not write like that.

[ralmsdeveloper]

So we have another problem.
For sample:

var @events = db
    .Events
    .Where(p => string.IsNullOrEmpty(p.Description))
    .ToArray();

 SELECT [e].[Id], [e].[DateOfEvent], [e].[Description], [e].[Key], [e].[Place], [e].[Price]
 FROM [Events] AS [e]
WHERE [e].[Description] IS NULL OR (([e].[Description] = N'') AND [e].[Description] IS NOT NULL)

[smitpatel]

If you do not add that term then [e].[Description] = N'VSSUMMIT' would give null back if it encounters null value. Where Null will skip the row so yes, your results would be right in this case. But for projection, it would fail. Your client side column will be bool type but value you get from server is null. So you get an exception. So additional term makes it fully null safe and convert it from 3 value logic to 2 value logic. We are aware that in case of predicate, we can get away with full expansion since null and false are equivalent, we decided to not bother that distinction for 3.0 since functionally it is working correctly.

[ralmsdeveloper]

@smitpatel, you guys did a great job rewriting the query pipeline, I wanted to do this provocation because many will question this.

This is more process on the server, and if it is not an index field, this doubles the processing value.

Thanks for the answers.

[ajcvickers]

@maumar to de-dupe

[maumar]

turns out we didn't have a tracking issue for this improvement (thought I filed one) - will use this issue to track instead

[mburbea]

May I suggest the following translation for sql server?
WHERE EXISTS (SELECT [e].[Description] INTERSECT SELECT N'VSSUMMIT')
This generates an = comparison for sql server but with is compare mode rather than eq compare mode.
This is equivalent x is distinct from y that is available in some variants of SQL (but not sql server).

This has the added benefit that you can generate cleaner sql for complicated clauses.
e.g. instead of having to generate

where  (t.[a] = @a or (t.[a] is null and and @a is null))
           and (t.[b] = @b or (t.[b] is null and @b is null))
           and (t.[c] = @c or (t.[c] is null and  @c is null))

You can generate the much simpler

where exists (select t.[a],t.[b],t.[c] intersect select @a,@b,@c)

These two statements both get the same exact same plan, which when you inspect the xml will show the following.

<ScalarOperator ScalarString="[db].[dbo].[t].[a] = [@a] 
AND [db].[dbo].[t].[b] = [@b] 
AND [db].[dbo].[t].[c] = [@c]">
<Logical Operation="AND">
<ScalarOperator>
<Compare CompareOp="IS">
...

https://dbfiddle.uk/?rdbms=sqlserver_2017&fiddle=388214ea04487d42dcab3ff8e25b3245

Now, this is admittedly a query very few would actually write, but it does generate the exact same plan (That is safe) and is less verbose and ugly and is the proper solution to coalesce issues that few want to deal with.

[maumar]

this has been fixed in 3.1, closing as dupe of #17543