Invalid date parameter format with nullables and MappedAs

Description

In some circumstances, the Linq to NHibernate provider emits datetime parameters as string literals formatted to short date time string, using current locales.
This may result in an invalid date emitted in the SQL.

This trouble happens when comparing a nullable DateTime? property of an entity to a DateTime value with its type overloaded through MappedAs to a date/datetime/timestamp type.

var testDate = new DateTime(2017, 03, 15); session.Query<Entity>() .Where(e => e.NullableDateTime == testDate.MappedAs(NHibernateUtil.Date))

Does emit with French locales:

select entity0_.Id as Id1_0_, entity0_.Name as Name2_0_, entity0_.NonNullableDateTime as NonNul3_0_, entity0_.NullableDateTime as Nullab4_0_ from Entity entity0_ where entity0_.NullableDateTime='15/03/2017 00:00:00'

This fails if the SQL user language is English.
The same query on a non nullable property yields a quite different and more robust SQL:

exec sp_executesql N' select entity0_.Id as Id1_0_, entity0_.Name as Name2_0_, entity0_.NonNullableDateTime as NonNul3_0_, entity0_.NullableDateTime as Nullab4_0_ from Entity entity0_ where entity0_.NonNullableDateTime=@p0',N'@p0 datetime',@p0='2017-03-15 00:00:00'

Environment

MS Sql Server, french locales, english sql user

is related to

Activity

Frédéric Delaporte March 17, 2017 at 4:39 PM

PR done.

Frédéric Delaporte March 17, 2017 at 3:12 PM

The expression constant seen at HQL tree construction is still a DateTime? because a second partial expression tree evaluation occurs from NhRelinqQueryParser, after the parameters map build in NhLinqExpression, when its Translate method is called.
The parameters map build alters the expression by removing the non "evaluatable" MappedAs call, yielding instead its first parameter value (the DateTime in this case). This causes the wrapping convert to nullable to be "evaluatable", while it was not when the MappedAs was still there.

Then the second partial expression tree evaluation further alter the expression, transforming the DateTime constant to a DateTime?, causing it to be no more "parameterized".

All that is worsen by the fact that the expression Key has already been computed before this second evaluation, causing it to ignore the DateTime value since it is supposed to be "parameterized". This causes the additional parameter value caching bug.

The NhRelinqQueryParser partial expression tree evaluation seems unneeded, since it is used solely by NhLinqExpression, which has already done it within its constructor. It looks safe to me to remove it (and all tests passes, going to PR that). Unfortunately, this may be a possible breaking change for some external library, since NhRelinqQueryParser is public and could have other usages outside of NHibernate.

Frédéric Delaporte March 16, 2017 at 6:05 PM
Edited

It seems the root of the trouble is the comparison of a "to be parameterized" DateTime to a nullable property: the resulting expression tree wrap the DateTime inside a convert causing it to be identified as a parameter candidate with a nullable DateTime? type.
But when we additionally call MappedAs on it, the wrapping convert can no more be evaluated by Re-Linq since MappedAs cannot be evaluated. This causes somehow (still have to identify where precisely) a parameter type mismatch: the ConstantToParameterMap ends up filled with a non nullable DateTime key (in ExpressionParameterVisitor.VisitMethodCallExpression) while the hql tree constant expression is still a nullable DateTime? (not determined yet from where it comes). This causes it to be not parameterized in HqlGeneratorExpressionTreeVisitor.VisitConstantExpression.

I wonder if we should strive for eliminating this mismatch, or if we should support it by making ConstantToParameterMap considers such values (one nullable, the other not, but having the same value indeed) equals.

Frédéric Delaporte March 15, 2017 at 7:04 PM

Well, there is actually a parameter value caching bug, but its reproduction conditions are so closed to those of this https://nhibernate.jira.com/browse/NH-3961#icft=NH-3961 bug (requires comparison to nullable property and usage of MappedAs) that it is very likely to have the same root cause.

So I have just added some additional test cases in this https://nhibernate.jira.com/browse/NH-3961#icft=NH-3961 bug.

Frédéric Delaporte March 15, 2017 at 5:58 PM

Test cases available here.

I intend to try fix this, but not today. I think I have found another more critical bug while elaborating those test cases, something similar to https://nhibernate.jira.com/browse/NH-3247#icft=NH-3247 but with datetime parameters. I will try to sort this out first (and then issue another Jira if I end up confirming this another issue).

Fixed

Details

Assignee

Reporter

Labels

Components

Fix versions

Affects versions

Priority

Who's Looking?

Open Who's Looking?
Created March 15, 2017 at 5:39 PM
Updated July 11, 2017 at 12:08 PM
Resolved March 19, 2017 at 8:15 AM
Configure
Who's Looking?