Linq provider generates different SQL query (with not equal) in version 4 and 5

[rspacjer]

For those two tests:

[Test]
public void Test_where_not_equal()
{
        var q = from p in db.Products
        where p.ProductId != int.MaxValue
        select new { p.ProductId, p.UnitPrice, HalfPrice = p.UnitPrice + 2 };

        var l = q.ToList();
}

[Test]
public void Test_where_not_equal_query_over()
{
        var q = _session.QueryOver<Product>().Where(x => x.ProductId != int.MaxValue);

        var l = q.List();
}

nHibernate 4.1.x generates such SQL:

    select
        product0_.ProductId as col_0_0_,
        product0_.UnitPrice as col_1_0_,
        product0_.UnitPrice as col_2_0_ 
    from
        Products product0_ 
    where
        product0_.ProductId<>@p0;
    @p0 = 2147483647

and

SELECT
        this_.ProductId as Produ1_32_0_,
        this_.ProductName as Produ2_32_0_,
        this_.SupplierId as Suppl3_32_0_,
        this_.CategoryId as Categ4_32_0_,
        this_.QuantityPerUnit as Quant5_32_0_,
        this_.UnitPrice as UnitP6_32_0_,
        this_.UnitsInStock as Units7_32_0_,
        this_.UnitsOnOrder as Units8_32_0_,
        this_.ReorderLevel as Reord9_32_0_,
        this_.Discontinued as Disc10_32_0_,
        this_.ShippingWeight as Ship11_32_0_ 
    FROM
        Products this_ 
    WHERE
        not (this_.ProductId = @p0);
    @p0 = 2147483647

and nHibernate 5.0.3 generates such SQL:

    select
        product0_.ProductId as col_0_0_,
        product0_.UnitPrice as col_1_0_,
        product0_.UnitPrice as col_2_0_ 
    from
        Products product0_ 
    where
        product0_.ProductId<>@p0 
        or product0_.ProductId is null;
    @p0 = 2147483647

and

    SELECT
        this_.ProductId as productid1_32_0_,
        this_.ProductName as productname2_32_0_,
        this_.SupplierId as supplierid3_32_0_,
        this_.CategoryId as categoryid4_32_0_,
        this_.QuantityPerUnit as quantityperunit5_32_0_,
        this_.UnitPrice as unitprice6_32_0_,
        this_.UnitsInStock as unitsinstock7_32_0_,
        this_.UnitsOnOrder as unitsonorder8_32_0_,
        this_.ReorderLevel as reorderlevel9_32_0_,
        this_.Discontinued as discontinued10_32_0_,
        this_.ShippingWeight as shippingweight11_32_0_ 
    FROM
        Products this_ 
    WHERE
        not (this_.ProductId = @p0);
    @p0 = 2147483647

As you can see, there is a difference for the Linq provider between version 4 and 5 in the where clauses.
For version 4 we have: product0_.ProductId<>@p0; and for 5 we have: product0_.ProductId<>@p0 or product0_.ProductId is null; This change makes that returned results can be different (I would event thread it as a breaking change between version 4 and 5). Is this by intention or is it a bug?

You can also notice that for version 5 the result returned by Ling and QueryOver is different, because those queries differs in the where statement (the QueryOver part doesn't have or product0_.ProductId is null)

Thanks in advance!

[fredericDelaporte]

This appears to be an undocumented breaking change from NH-3100 correction (#516), where the Linq "not equal" case does now handle nullability (added here).

This looks clearly as being by intention.

About whether the discrepancy between Linq and QueryOver should be addressed or not, I have no opinion. In QueryOver lambda are here mainly for supplying property names to the underlying Criteria. The support of such a Where is an added convenience feature, not meant to mimic Linq semantic. For deciding whether it should be changed too or not, it should be checked if that does make sens in regard to the equivalent Criteria semantic. As I am not a Criteria/QueryOver user, I do not know.

[hazzik]

Is this by intention or is it a bug?

It's intentional. Please check the changes to NullComparationTests.cs

We need to add a "possible breaking change" note into the release notes.

The Linq semantic should match across providers (Linq-to-objects, NH, EF, etc), and this case was a clear bug. So it needs to be a little sophisticated about how to translate the expression tree to the SQL. The QueryOver, on the other hand, is dumber and requires more explicit expressions. But these query providers are not directly comparable.

I think as a probable solution we could make IsNullable check in TranslateInequalityComparison more clever to detect nullable types.

Please provide a test which demonstrates that there are incorrect results can be yielded.

[rspacjer]

Thanks a lot for clarification. Now I can fix my code, because I'm sure it's not a bug, but intended change.

regards