Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix to #18871 - Support for indirect joins which are using non-equal conditions #21397

Merged
merged 3 commits into from
Jul 9, 2020
Merged

Fix to #18871 - Support for indirect joins which are using non-equal conditions #21397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a70fde4
add arithmetic operators for Sqlite
lokalmatador Mar 6, 2020
2eacef0
adding and fixing tests
maumar Jul 9, 2020
9e63656
Fix to #18871 - Support for indirect joins which are using non-equal …
maumar Jun 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 39 additions & 10 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions
// Class is sealed because there are no public/protected constructors. Can be unsealed if this is changed.
public sealed class SelectExpression : TableExpressionBase
{
private static readonly Dictionary<ExpressionType, ExpressionType> _mirroredOperationMap = new Dictionary<ExpressionType, ExpressionType>
{
{ ExpressionType.Equal, ExpressionType.Equal },
{ ExpressionType.NotEqual, ExpressionType.NotEqual },
{ ExpressionType.LessThan, ExpressionType.GreaterThan },
{ ExpressionType.LessThanOrEqual, ExpressionType.GreaterThanOrEqual },
{ ExpressionType.GreaterThan, ExpressionType.LessThan },
{ ExpressionType.GreaterThanOrEqual, ExpressionType.LessThanOrEqual },
};

private readonly IDictionary<EntityProjectionExpression, IDictionary<IProperty, int>> _entityProjectionCache
= new Dictionary<EntityProjectionExpression, IDictionary<IProperty, int>>();

Expand Down Expand Up @@ -1566,14 +1576,20 @@ private static SqlExpression MakeNullable(SqlExpression sqlExpression)
return (NewArrayInit(typeof(object), updatedExpressions), comparers);
}

private SqlExpression TryExtractJoinKey(SelectExpression selectExpression)
private SqlExpression TryExtractJoinKey(SelectExpression selectExpression, bool allowNonEquality)
{
if (selectExpression.Limit == null
&& selectExpression.Offset == null
&& selectExpression.Predicate != null)
{
var columnExpressions = new List<ColumnExpression>();
var joinPredicate = TryExtractJoinKey(selectExpression, selectExpression.Predicate, columnExpressions, out var predicate);
var joinPredicate = TryExtractJoinKey(
selectExpression,
selectExpression.Predicate,
columnExpressions,
allowNonEquality,
out var predicate);

if (joinPredicate != null)
{
joinPredicate = RemoveRedundantNullChecks(joinPredicate, columnExpressions);
Expand All @@ -1591,11 +1607,12 @@ private SqlExpression TryExtractJoinKey(
SelectExpression selectExpression,
SqlExpression predicate,
List<ColumnExpression> columnExpressions,
bool allowNonEquality,
out SqlExpression updatedPredicate)
{
if (predicate is SqlBinaryExpression sqlBinaryExpression)
{
var joinPredicate = ValidateKeyComparison(selectExpression, sqlBinaryExpression, columnExpressions);
var joinPredicate = ValidateKeyComparison(selectExpression, sqlBinaryExpression, columnExpressions, allowNonEquality);
if (joinPredicate != null)
{
updatedPredicate = null;
Expand All @@ -1606,9 +1623,9 @@ private SqlExpression TryExtractJoinKey(
if (sqlBinaryExpression.OperatorType == ExpressionType.AndAlso)
{
var leftJoinKey = TryExtractJoinKey(
selectExpression, sqlBinaryExpression.Left, columnExpressions, out var leftPredicate);
selectExpression, sqlBinaryExpression.Left, columnExpressions, allowNonEquality, out var leftPredicate);
var rightJoinKey = TryExtractJoinKey(
selectExpression, sqlBinaryExpression.Right, columnExpressions, out var rightPredicate);
selectExpression, sqlBinaryExpression.Right, columnExpressions, allowNonEquality, out var rightPredicate);

updatedPredicate = CombineNonNullExpressions(leftPredicate, rightPredicate);

Expand All @@ -1629,9 +1646,18 @@ private static SqlExpression CombineNonNullExpressions(SqlExpression left, SqlEx
: right;

private SqlBinaryExpression ValidateKeyComparison(
SelectExpression inner, SqlBinaryExpression sqlBinaryExpression, List<ColumnExpression> columnExpressions)
SelectExpression inner,
SqlBinaryExpression sqlBinaryExpression,
List<ColumnExpression> columnExpressions,
bool allowNonEquality)
{
if (sqlBinaryExpression.OperatorType == ExpressionType.Equal)
if (sqlBinaryExpression.OperatorType == ExpressionType.Equal
|| (allowNonEquality &&
(sqlBinaryExpression.OperatorType == ExpressionType.NotEqual
|| sqlBinaryExpression.OperatorType == ExpressionType.GreaterThan
|| sqlBinaryExpression.OperatorType == ExpressionType.GreaterThanOrEqual
|| sqlBinaryExpression.OperatorType == ExpressionType.LessThan
|| sqlBinaryExpression.OperatorType == ExpressionType.LessThanOrEqual)))
{
if (sqlBinaryExpression.Left is ColumnExpression leftColumn
&& sqlBinaryExpression.Right is ColumnExpression rightColumn)
Expand All @@ -1649,9 +1675,12 @@ private SqlBinaryExpression ValidateKeyComparison(
{
columnExpressions.Add(rightColumn);

return sqlBinaryExpression.Update(
return new SqlBinaryExpression(
_mirroredOperationMap[sqlBinaryExpression.OperatorType],
sqlBinaryExpression.Right,
sqlBinaryExpression.Left);
sqlBinaryExpression.Left,
sqlBinaryExpression.Type,
sqlBinaryExpression.TypeMapping);
}
}
}
Expand Down Expand Up @@ -2024,7 +2053,7 @@ private void AddJoin(
innerSelectExpression.Limit = null;
innerSelectExpression.Offset = null;

joinPredicate = TryExtractJoinKey(innerSelectExpression);
joinPredicate = TryExtractJoinKey(innerSelectExpression, allowNonEquality: limit == null && offset == null);
if (joinPredicate != null)
{
var containsOuterReference = new SelectExpressionCorrelationFindingExpressionVisitor(this)
Expand Down
7 changes: 6 additions & 1 deletion src/EFCore.Relational/Query/SqlNullabilityProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,12 @@ private SqlExpression ProcessJoinPredicate(SqlExpression predicate)
return result;
}

if (sqlBinaryExpression.OperatorType == ExpressionType.AndAlso)
if (sqlBinaryExpression.OperatorType == ExpressionType.AndAlso
|| sqlBinaryExpression.OperatorType == ExpressionType.NotEqual
|| sqlBinaryExpression.OperatorType == ExpressionType.GreaterThan
|| sqlBinaryExpression.OperatorType == ExpressionType.GreaterThanOrEqual
|| sqlBinaryExpression.OperatorType == ExpressionType.LessThan
|| sqlBinaryExpression.OperatorType == ExpressionType.LessThanOrEqual)
{
return Visit(sqlBinaryExpression, allowOptimizedExpansion: true, out _);
}
Expand Down
91 changes: 73 additions & 18 deletions src/EFCore.Sqlite.Core/Query/Internal/SqliteSqlTranslatingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,9 @@ private static readonly IReadOnlyDictionary<ExpressionType, IReadOnlyCollection<
{
typeof(DateTime),
typeof(DateTimeOffset),
typeof(decimal),
typeof(TimeSpan)
},
[ExpressionType.Divide] = new HashSet<Type>
{
typeof(decimal),
typeof(TimeSpan),
typeof(ulong)
},
[ExpressionType.Divide] = new HashSet<Type> { typeof(TimeSpan), typeof(ulong) },
[ExpressionType.GreaterThan] = new HashSet<Type>
{
typeof(DateTimeOffset),
Expand All @@ -63,17 +57,11 @@ private static readonly IReadOnlyDictionary<ExpressionType, IReadOnlyCollection<
typeof(ulong)
},
[ExpressionType.Modulo] = new HashSet<Type> { typeof(ulong) },
[ExpressionType.Multiply] = new HashSet<Type>
{
typeof(decimal),
typeof(TimeSpan),
typeof(ulong)
},
[ExpressionType.Multiply] = new HashSet<Type> { typeof(TimeSpan), typeof(ulong) },
[ExpressionType.Subtract] = new HashSet<Type>
{
typeof(DateTime),
typeof(DateTimeOffset),
typeof(decimal),
typeof(TimeSpan)
}
};
Expand Down Expand Up @@ -132,8 +120,17 @@ protected override Expression VisitUnary(UnaryExpression unaryExpression)
&& sqlUnary.OperatorType == ExpressionType.Negate)
{
var operandType = GetProviderType(sqlUnary.Operand);
if (operandType == typeof(decimal)
|| operandType == typeof(TimeSpan))
if (operandType == typeof(decimal))
{
return Dependencies.SqlExpressionFactory.Function(
name: "ef_negate",
new[] { sqlUnary.Operand },
nullable: true,
new[] { true },
visitedExpression.Type);
}

if (operandType == typeof(TimeSpan))
{
return null;
}
Expand Down Expand Up @@ -177,6 +174,11 @@ protected override Expression VisitBinary(BinaryExpression binaryExpression)
return DoDecimalCompare(visitedExpression, sqlBinary.OperatorType, sqlBinary.Left, sqlBinary.Right);
}

if (AttemptDecimalArithmetic(sqlBinary))
{
return DoDecimalArithmetics(visitedExpression, sqlBinary.OperatorType, sqlBinary.Left, sqlBinary.Right);
}

if (_restrictedBinaryExpressions.TryGetValue(sqlBinary.OperatorType, out var restrictedTypes)
&& (restrictedTypes.Contains(GetProviderType(sqlBinary.Left))
|| restrictedTypes.Contains(GetProviderType(sqlBinary.Right))))
Expand Down Expand Up @@ -291,9 +293,11 @@ private static Type GetProviderType(SqlExpression expression)
?? expression.TypeMapping?.ClrType
?? expression.Type).UnwrapNullableType();

private static bool AreOperandsDecimals(SqlBinaryExpression sqlExpression) => GetProviderType(sqlExpression.Left) == typeof(decimal)
&& GetProviderType(sqlExpression.Right) == typeof(decimal);

private static bool AttemptDecimalCompare(SqlBinaryExpression sqlBinary) =>
GetProviderType(sqlBinary.Left) == typeof(decimal)
&& GetProviderType(sqlBinary.Right) == typeof(decimal)
AreOperandsDecimals(sqlBinary)
&& new[]
{
ExpressionType.GreaterThan, ExpressionType.GreaterThanOrEqual, ExpressionType.LessThan, ExpressionType.LessThanOrEqual
Expand All @@ -318,5 +322,56 @@ private Expression DoDecimalCompare(SqlExpression visitedExpression, ExpressionT
_ => visitedExpression
};
}

private static bool AttemptDecimalArithmetic(SqlBinaryExpression sqlBinary) =>
AreOperandsDecimals(sqlBinary)
&& new[] { ExpressionType.Add, ExpressionType.Subtract, ExpressionType.Multiply, ExpressionType.Divide }.Contains(
sqlBinary.OperatorType);

private Expression DoDecimalArithmetics(SqlExpression visitedExpression, ExpressionType op, SqlExpression left, SqlExpression right)
{
return op switch
{
ExpressionType.Add => DecimalArithmeticExpressionFactoryMethod(ResolveFunctionNameFromExpressionType(op), left, right),
ExpressionType.Divide => DecimalArithmeticExpressionFactoryMethod(ResolveFunctionNameFromExpressionType(op), left, right),
ExpressionType.Multiply => DecimalArithmeticExpressionFactoryMethod(ResolveFunctionNameFromExpressionType(op), left, right),
ExpressionType.Subtract => DecimalSubtractExpressionFactoryMethod(left, right),
_ => visitedExpression
};

static string ResolveFunctionNameFromExpressionType(ExpressionType expressionType)
{
return expressionType switch
{
ExpressionType.Add => "ef_add",
ExpressionType.Divide => "ef_divide",
ExpressionType.Multiply => "ef_multiply",
ExpressionType.Subtract => "ef_add",
_ => throw new InvalidOperationException()
};
}

Expression DecimalArithmeticExpressionFactoryMethod(string name, SqlExpression left, SqlExpression right)
{
return Dependencies.SqlExpressionFactory.Function(
name,
new[] { left, right },
nullable: true,
new[] { true, true },
visitedExpression.Type);
}

Expression DecimalSubtractExpressionFactoryMethod(SqlExpression left, SqlExpression right)
{
var subtrahend = Dependencies.SqlExpressionFactory.Function(
"ef_negate",
new[] { right },
nullable: true,
new[] { true },
visitedExpression.Type);

return DecimalArithmeticExpressionFactoryMethod(ResolveFunctionNameFromExpressionType(op), left, subtrahend);
}
}
}
}
21 changes: 21 additions & 0 deletions src/EFCore.Sqlite.Core/Storage/Internal/SqliteRelationalConnection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,35 @@ private void InitializeDbConnection(DbConnection connection)
% Convert.ToDouble(divisor, CultureInfo.InvariantCulture);
});

sqliteConnection.CreateFunction(
name: "ef_add",
(decimal? left, decimal? right) => left + right,
isDeterministic: true);

sqliteConnection.CreateFunction(
name: "ef_divide",
(decimal? dividend, decimal? divisor) => dividend / divisor,
isDeterministic: true);

sqliteConnection.CreateFunction(
name: "ef_compare",
(decimal? left, decimal? right) => left.HasValue && right.HasValue
? decimal.Compare(left.Value, right.Value)
: default(int?),
isDeterministic: true);

sqliteConnection.CreateFunction(
name: "ef_multiply",
(decimal? left, decimal? right) => left * right,
isDeterministic: true);

sqliteConnection.CreateFunction(
name: "ef_negate",
(decimal? m) => -m,
isDeterministic: true);
}
else

{
_logger.UnexpectedConnectionTypeWarning(connection.GetType());
}
Expand Down
6 changes: 6 additions & 0 deletions test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2651,6 +2651,12 @@ FROM root c
WHERE ((c[""Discriminator""] = ""Customer"") AND (c[""City""] = ""Seattle""))");
}

[ConditionalTheory(Skip = "Issue#17246")]
public override Task DefaultIfEmpty_in_subquery_nested_filter_order_comparison(bool async)
{
return base.DefaultIfEmpty_in_subquery_nested_filter_order_comparison(async);
}

[ConditionalTheory(Skip = "Issue #17246")]
public override async Task OrderBy_skip_take(bool async)
{
Expand Down
18 changes: 18 additions & 0 deletions test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSelectQueryCosmosTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,24 @@ public override Task SelectMany_correlated_with_outer_4(bool async)
return base.SelectMany_correlated_with_outer_4(async);
}

[ConditionalTheory(Skip = "Issue#17246")]
public override Task SelectMany_correlated_with_outer_5(bool async)
{
return base.SelectMany_correlated_with_outer_5(async);
}

[ConditionalTheory(Skip = "Issue#17246")]
public override Task SelectMany_correlated_with_outer_6(bool async)
{
return base.SelectMany_correlated_with_outer_6(async);
}

[ConditionalTheory(Skip = "Issue#17246")]
public override Task SelectMany_correlated_with_outer_7(bool async)
{
return base.SelectMany_correlated_with_outer_7(async);
}

[ConditionalTheory(Skip = "Issue#17246")]
public override Task FirstOrDefault_over_empty_collection_of_value_type_returns_correct_results(bool async)
{
Expand Down
4 changes: 4 additions & 0 deletions test/EFCore.InMemory.FunctionalTests/Query/GearsOfWarQueryInMemoryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,9 @@ public override Task Client_member_and_unsupported_string_Equals_in_the_same_que
[ConditionalTheory(Skip = "issue #17386")]
public override Task Client_eval_followed_by_set_operation_throws_meaningful_exception(bool async)
=> base.Client_eval_followed_by_set_operation_throws_meaningful_exception(async);

[ConditionalTheory(Skip = "issue #17537")]
public override Task SelectMany_predicate_with_non_equality_comparison_with_Take_doesnt_convert_to_join(bool async)
=> base.SelectMany_predicate_with_non_equality_comparison_with_Take_doesnt_convert_to_join(async);
}
}
Loading