Open dallmair opened 1 month ago
Thinking about this some more, the existing caching is necessary for correctness due to NULL propagation semantics. For example, A() == B()
must be compiled to:
var temp1 = A();
var temp2 = B();
return temp1 == null || temp2 == null ? null : temp1 == temp2;
Without caching, the evaluation would happen twice, which is at best undesired and at worst incorrect (when the function has side effects).
Thus, it makes sense to keep the general approach, and fix the problem in the compilation of CASE WHEN
, because that is the only expression in SQL that has lazy-evaluation semantics.
Background
We have a table similar to:
We're trying to transform the date value to a different format using NQuery with:
Expected result
Returns table with date value transformed to
2023-07-27
.Actual result
Throws exception:
Analysis
The problem is caused by the caching of expression values in variables implemented in
ExpressionBuilder.BuildCachedExpression
. This code transforms expressions likea.b.c.Prop
toAnd it transforms our
CASE WHEN Type = 'date' THEN FORMAT(TO_DATETIME(Value), 'yyyy-MM-dd') ELSE Value END
to:In words: It pulls calculations out of the
CASE
expression, changing the semantics (searchedCASE
expressions are short-circuited in SQL).Open questions
As a little experiment, I removed caching by replacing all calls to
BuildCachedExpression
withBuildLiftedExpression
. All unit tests are still green, but I'm unsure whether that is a good idea as I do not know:CASE WHEN
bug described here?Highly appreciate any insights.