-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix to #18555 - Query: when rewriting null semantics for comparisons with functions use function specific metadata to get better SQL #19607
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -898,8 +898,7 @@ private SqlExpression RewriteNullSemantics( | |
return sqlBinaryExpression.Update(left, right); | ||
} | ||
|
||
private SqlExpression SimplifyLogicalSqlBinaryExpression( | ||
SqlBinaryExpression sqlBinaryExpression) | ||
private SqlExpression SimplifyLogicalSqlBinaryExpression(SqlBinaryExpression sqlBinaryExpression) | ||
{ | ||
var leftUnary = sqlBinaryExpression.Left as SqlUnaryExpression; | ||
var rightUnary = sqlBinaryExpression.Right as SqlUnaryExpression; | ||
|
@@ -1253,37 +1252,96 @@ protected virtual SqlExpression ProcessNullNotNull( | |
sqlUnaryExpression.TypeMapping)); | ||
} | ||
|
||
case SqlFunctionExpression sqlFunctionExpression | ||
when sqlFunctionExpression.IsBuiltIn && string.Equals("COALESCE", sqlFunctionExpression.Name, StringComparison.OrdinalIgnoreCase): | ||
case SqlFunctionExpression sqlFunctionExpression: | ||
{ | ||
// for coalesce: | ||
// (a ?? b) == null -> a == null && b == null | ||
// (a ?? b) != null -> a != null || b != null | ||
var left = ProcessNullNotNull( | ||
SqlExpressionFactory.MakeUnary( | ||
sqlUnaryExpression.OperatorType, | ||
sqlFunctionExpression.Arguments[0], | ||
typeof(bool), | ||
sqlUnaryExpression.TypeMapping), | ||
operandNullable: null); | ||
if (sqlFunctionExpression.IsBuiltIn && string.Equals("COALESCE", sqlFunctionExpression.Name, StringComparison.OrdinalIgnoreCase)) | ||
{ | ||
// for coalesce: | ||
// (a ?? b) == null -> a == null && b == null | ||
// (a ?? b) != null -> a != null || b != null | ||
var left = ProcessNullNotNull( | ||
SqlExpressionFactory.MakeUnary( | ||
sqlUnaryExpression.OperatorType, | ||
sqlFunctionExpression.Arguments[0], | ||
typeof(bool), | ||
sqlUnaryExpression.TypeMapping), | ||
operandNullable: null); | ||
|
||
var right = ProcessNullNotNull( | ||
SqlExpressionFactory.MakeUnary( | ||
sqlUnaryExpression.OperatorType, | ||
sqlFunctionExpression.Arguments[1], | ||
typeof(bool), | ||
sqlUnaryExpression.TypeMapping), | ||
operandNullable: null); | ||
|
||
var right = ProcessNullNotNull( | ||
SqlExpressionFactory.MakeUnary( | ||
sqlUnaryExpression.OperatorType, | ||
sqlFunctionExpression.Arguments[1], | ||
typeof(bool), | ||
sqlUnaryExpression.TypeMapping), | ||
operandNullable: null); | ||
return SimplifyLogicalSqlBinaryExpression( | ||
SqlExpressionFactory.MakeBinary( | ||
sqlUnaryExpression.OperatorType == ExpressionType.Equal | ||
? ExpressionType.AndAlso | ||
: ExpressionType.OrElse, | ||
left, | ||
right, | ||
sqlUnaryExpression.TypeMapping)); | ||
} | ||
|
||
return SimplifyLogicalSqlBinaryExpression( | ||
SqlExpressionFactory.MakeBinary( | ||
sqlUnaryExpression.OperatorType == ExpressionType.Equal | ||
? ExpressionType.AndAlso | ||
: ExpressionType.OrElse, | ||
left, | ||
right, | ||
sqlUnaryExpression.TypeMapping)); | ||
if (!sqlFunctionExpression.NullResultAllowed) | ||
{ | ||
// when we know that function can't be nullable: | ||
// non_nullable_function() is null-> false | ||
// non_nullable_function() is not null -> true | ||
return SqlExpressionFactory.Constant( | ||
sqlUnaryExpression.OperatorType == ExpressionType.NotEqual, | ||
sqlUnaryExpression.TypeMapping); | ||
} | ||
|
||
// see if we can derive function nullability from it's instance and/or arguments | ||
// rather than evaluating nullability of the entire function | ||
var nullabilityPropagationElements = new List<SqlExpression>(); | ||
if (sqlFunctionExpression.Instance != null | ||
&& sqlFunctionExpression.InstancPropagatesNullability == true) | ||
{ | ||
nullabilityPropagationElements.Add(sqlFunctionExpression.Instance); | ||
} | ||
|
||
for (var i = 0; i < sqlFunctionExpression.Arguments.Count; i++) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check if is niladic to avoid for loop. |
||
{ | ||
if (sqlFunctionExpression.ArgumentsPropagateNullability[i]) | ||
{ | ||
nullabilityPropagationElements.Add(sqlFunctionExpression.Arguments[i]); | ||
} | ||
} | ||
|
||
if (nullabilityPropagationElements.Count > 0) | ||
{ | ||
var result = ProcessNullNotNull( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using aggregate could be cleaner. |
||
SqlExpressionFactory.MakeUnary( | ||
sqlUnaryExpression.OperatorType, | ||
nullabilityPropagationElements[0], | ||
sqlUnaryExpression.Type, | ||
sqlUnaryExpression.TypeMapping), | ||
operandNullable: null); | ||
|
||
foreach (var element in nullabilityPropagationElements.Skip(1)) | ||
{ | ||
result = SimplifyLogicalSqlBinaryExpression( | ||
sqlUnaryExpression.OperatorType == ExpressionType.Equal | ||
? SqlExpressionFactory.OrElse( | ||
result, | ||
ProcessNullNotNull( | ||
SqlExpressionFactory.IsNull(element), | ||
operandNullable: null)) | ||
: SqlExpressionFactory.AndAlso( | ||
result, | ||
ProcessNullNotNull( | ||
SqlExpressionFactory.IsNotNull(element), | ||
operandNullable: null))); | ||
} | ||
|
||
return result; | ||
} | ||
} | ||
break; | ||
} | ||
|
||
return sqlUnaryExpression; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,6 +56,8 @@ public virtual SqlExpression Translate( | |
dbFunction.Schema, | ||
dbFunction.Name, | ||
arguments, | ||
nullResultAllowed: true, | ||
argumentsPropagateNullability: arguments.Select(a => true).ToList(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. default should be false. |
||
method.ReturnType); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
canBeNull would be better name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will go with nullable/isnullable, just like column expression