Skip to content

Commit

Permalink
Translate parameterized Contains to PG = ANY ()
Browse files Browse the repository at this point in the history 
Closes #916
  • Loading branch information
@roji
roji committed Nov 5, 2019
1 parent 028a2dc commit d7863b8
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 43 deletions.
63 changes: 43 additions & 20 deletions src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlArrayMethodTranslator.cs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
Expand Down Expand Up @@ -46,23 +47,33 @@ public SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadO
{
// TODO: Fully support List<>

if (arguments.Count == 0 || !arguments[0].Type.IsArray &&
(!arguments[0].Type.IsGenericType || arguments[0].Type.GetGenericTypeDefinition() != typeof(List<>)))
if (arguments.Count == 0)
return null;

var arrayOperand = arguments[0];
var operand = arguments[0];
Type operandElementType;

if (operand.Type.IsArray)
{
operandElementType = operand.Type.GetElementType();
}
else if (operand.Type.IsGenericType && operand.Type.GetGenericTypeDefinition() == typeof(List<>))
{
operandElementType = operand.Type.GetGenericArguments()[0];
}
else
return null; // Not an array/list

if (method.IsClosedFormOf(SequenceEqual) && arguments[1].Type.IsArray)
return _sqlExpressionFactory.Equal(arrayOperand, arguments[1]);
return _sqlExpressionFactory.Equal(operand, arguments[1]);

// Predicate-less Any - translate to a simple length check.
if (method.IsClosedFormOf(EnumerableAnyWithoutPredicate))
{
return
_sqlExpressionFactory.GreaterThan(
_jsonPocoTranslator.TranslateArrayLength(arrayOperand) ??
_sqlExpressionFactory.Function("cardinality", arguments, typeof(int?)),
_sqlExpressionFactory.Constant(0));
return _sqlExpressionFactory.GreaterThan(
_jsonPocoTranslator.TranslateArrayLength(operand) ??
_sqlExpressionFactory.Function("cardinality", arguments, typeof(int?)),
_sqlExpressionFactory.Constant(0));
}

// Note that .Where(e => new[] { "a", "b", "c" }.Any(p => e.SomeText == p)))
Expand All @@ -73,20 +84,32 @@ public SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadO
// since non-PG SQL does not support arrays. If the list is a constant we leave it for regular IN
// (functionality the same but more familiar).

// TODO: The following does not work correctly if there are any nulls in a parameterized array, because
// of null semantics (test: Contains_on_nullable_array_produces_correct_sql).
// https://github.com/aspnet/EntityFrameworkCore/issues/15892 tracks caching based on parameter values,
// which should allow us to enable this and have correct behavior.

// We still apply this translation when it's on a column expression, since that can't work anyway with
// EF Core's parameter to constant expansion
// Note: we exclude constant array expressions from this PG-specific optimization since the general
// EF Core mechanism is fine for that case. After https://github.com/aspnet/EntityFrameworkCore/issues/16375
// is done we may not need the check any more.
// Note: we exclude arrays/lists over Nullable<T> since the ADO layer doesn't handle them (but will in 5.0)

if (method.IsClosedFormOf(Contains) &&
arrayOperand is ColumnExpression &&
//!(arrayOperand is SqlConstantExpression) && // When the null semantics issue is resolved
_sqlExpressionFactory.FindMapping(arrayOperand.Type) != null)
_sqlExpressionFactory.FindMapping(operand.Type) != null &&
!(operand is SqlConstantExpression) &&
Nullable.GetUnderlyingType(operandElementType) == null)
{
return _sqlExpressionFactory.ArrayAnyAll(arguments[1], arrayOperand, ArrayComparisonType.Any, "=");
var item = arguments[1];
// We require a null semantics check in case the item is null and the array contains a null.
// Advanced parameter sniffing would help here: https://github.com/aspnet/EntityFrameworkCore/issues/17598
return _sqlExpressionFactory.OrElse(
// We need to coalesce to false since 'x' = ANY ({'y', NULL}) returns null, not false
// (and so will be null when negated too)
_sqlExpressionFactory.Coalesce(
_sqlExpressionFactory.ArrayAnyAll(item, operand, ArrayComparisonType.Any, "="),
_sqlExpressionFactory.Constant(false)),
_sqlExpressionFactory.AndAlso(
_sqlExpressionFactory.IsNull(item),
_sqlExpressionFactory.IsNotNull(
_sqlExpressionFactory.Function(
"array_position",
new[] { operand, _sqlExpressionFactory.Fragment("NULL") },
typeof(int)))));
}

// Note: we also translate .Where(e => new[] { "a", "b", "c" }.Any(p => EF.Functions.Like(e.SomeText, p)))
Expand Down
66 changes: 43 additions & 23 deletions test/EFCore.PG.FunctionalTests/Query/SimpleQueryNpgsqlTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,25 +76,26 @@ public override async Task Contains_with_local_uint_array_closure(bool isAsync)
{
await base.Contains_with_local_uint_array_closure(isAsync);

// This test invokes Contains() on a uint array, but PostgreSQL doesn't support uint. As a result,
// we can't do the PostgreSQL-specific x = ANY (a,b,c) optimization and allow EF Core to expand the
// parameterized array to constant instead.

// Note: PostgreSQL doesn't support uint, but value converters make this into bigint
AssertSql(
@"SELECT e.""EmployeeID"", e.""City"", e.""Country"", e.""FirstName"", e.""ReportsTo"", e.""Title""
@"@__ids_0='System.Int64[]' (DbType = Object)
SELECT e.""EmployeeID"", e.""City"", e.""Country"", e.""FirstName"", e.""ReportsTo"", e.""Title""
FROM ""Employees"" AS e
WHERE e.""EmployeeID"" IN (0, 1)",
WHERE COALESCE(e.""EmployeeID"" = ANY (@__ids_0), FALSE) OR ((e.""EmployeeID"" IS NULL) AND (array_position(@__ids_0, NULL) IS NOT NULL))",
//
@"SELECT e.""EmployeeID"", e.""City"", e.""Country"", e.""FirstName"", e.""ReportsTo"", e.""Title""
@"@__ids_0='System.Int64[]' (DbType = Object)
SELECT e.""EmployeeID"", e.""City"", e.""Country"", e.""FirstName"", e.""ReportsTo"", e.""Title""
FROM ""Employees"" AS e
WHERE e.""EmployeeID"" IN (0)");
WHERE COALESCE(e.""EmployeeID"" = ANY (@__ids_0), FALSE) OR ((e.""EmployeeID"" IS NULL) AND (array_position(@__ids_0, NULL) IS NOT NULL))");
}

public override async Task Contains_with_local_nullable_uint_array_closure(bool isAsync)
{
await base.Contains_with_local_nullable_uint_array_closure(isAsync);

// As above in Contains_with_local_int_array_closure
// Note: PostgreSQL doesn't support uint, but value converters make this into bigint

AssertSql(
@"SELECT e.""EmployeeID"", e.""City"", e.""Country"", e.""FirstName"", e.""ReportsTo"", e.""Title""
Expand Down Expand Up @@ -252,27 +253,46 @@ await AssertQuery(
[MemberData(nameof(IsAsyncData))]
public async Task Array_Contains_parameter(bool isAsync)
{
var ids = new[] { "ALFKI", "ANATR" };
var regions = new[] { "UK", "SP" };

await AssertQuery(
isAsync,
ss => ss.Set<Customer>().Where(c => ids.Contains(c.CustomerID)),
entryCount: 2);
ss => ss.Set<Customer>().Where(c => regions.Contains(c.Region)),
entryCount: 6);

// Instead of c.""Region"" IN ('UK', 'SP') we generate the PostgreSQL-specific x = ANY (a, b, c), which can
// be parameterized.
// Ideally parameter sniffing would allow us to produce SQL without the null check since the regions array doesn't contain one
// (see https://github.com/aspnet/EntityFrameworkCore/issues/17598).
AssertSql(
@"@__regions_0='System.String[]' (DbType = Object)
SELECT c.""CustomerID"", c.""Address"", c.""City"", c.""CompanyName"", c.""ContactName"", c.""ContactTitle"", c.""Country"", c.""Fax"", c.""Phone"", c.""PostalCode"", c.""Region""
FROM ""Customers"" AS c
WHERE COALESCE(c.""Region"" = ANY (@__regions_0), FALSE) OR ((c.""Region"" IS NULL) AND (array_position(@__regions_0, NULL) IS NOT NULL))");
}

// Instead of c.""CustomerID"" x in ('ALFKI', 'ANATR') we should generate the PostgreSQL-specific x = ANY (a, b, c), which can
// be parameterized. This is currently disabled because of null semantics, until EF Core supports caching
// based on parameter values (https://github.com/aspnet/EntityFrameworkCore/issues/15892#issuecomment-513399906).
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public async Task Array_Contains_parameter_with_null(bool isAsync)
{
var regions = new[] { "UK", "SP", null };

await AssertQuery(
isAsync,
ss => ss.Set<Customer>().Where(c => regions.Contains(c.Region)),
entryCount: 66);

// Instead of c.""Region"" IN ('UK', 'SP') we generate the PostgreSQL-specific x = ANY (a, b, c), which can
// be parameterized.
// Ideally parameter sniffing would allow us to produce SQL with an optimized null check (no need to check the array server-side)
// (see https://github.com/aspnet/EntityFrameworkCore/issues/17598).
AssertSql(
@"SELECT c.""CustomerID"", c.""Address"", c.""City"", c.""CompanyName"", c.""ContactName"", c.""ContactTitle"", c.""Country"", c.""Fax"", c.""Phone"", c.""PostalCode"", c.""Region""
@"@__regions_0='System.String[]' (DbType = Object)
SELECT c.""CustomerID"", c.""Address"", c.""City"", c.""CompanyName"", c.""ContactName"", c.""ContactTitle"", c.""Country"", c.""Fax"", c.""Phone"", c.""PostalCode"", c.""Region""
FROM ""Customers"" AS c
WHERE c.""CustomerID"" IN ('ALFKI', 'ANATR')");
// AssertSql(
// @"@__ids_0='System.String[]' (DbType = Object)
//
//SELECT c.""CustomerID"", c.""Address"", c.""City"", c.""CompanyName"", c.""ContactName"", c.""ContactTitle"", c.""Country"", c.""Fax"", c.""Phone"", c.""PostalCode"", c.""Region""
//FROM ""Customers"" AS c
//WHERE c.""CustomerID"" = ANY (@__ids_0)");
WHERE COALESCE(c.""Region"" = ANY (@__regions_0), FALSE) OR ((c.""Region"" IS NULL) AND (array_position(@__regions_0, NULL) IS NOT NULL))");
}

#endregion Array contains
Expand Down

0 comments on commit d7863b8

Please sign in to comment.