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

Remove COALESCE from array ANY expression #1163

Merged
merged 1 commit into from
Dec 13, 2019
Merged

Remove COALESCE from array ANY expression #1163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 4 additions & 5 deletions src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlArrayMethodTranslator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,13 @@ public SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadO
Nullable.GetUnderlyingType(operandElementType) == null)
{
var item = arguments[1];
// TODO: no null semantics is implemented here (see https://github.com/npgsql/efcore.pg/issues/1142)
// 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
// We need to coalesce to false since 'x' = ANY ({'y', NULL}) returns null, not false
// (and so will be null when negated too)
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.ArrayAnyAll(item, operand, ArrayComparisonType.Any, "="),
_sqlExpressionFactory.AndAlso(
_sqlExpressionFactory.IsNull(item),
_sqlExpressionFactory.IsNotNull(
Expand Down
6 changes: 3 additions & 3 deletions test/EFCore.PG.FunctionalTests/Query/ArrayQueryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public void Contains_with_literal()
AssertSql(
@"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText""
FROM ""SomeEntities"" AS s
WHERE COALESCE(3 = ANY (s.""SomeArray""), FALSE)
WHERE 3 = ANY (s.""SomeArray"")
LIMIT 2");
}

Expand All @@ -158,7 +158,7 @@ public void Contains_with_parameter()

SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText""
FROM ""SomeEntities"" AS s
WHERE COALESCE(@__p_0 = ANY (s.""SomeArray""), FALSE)
WHERE @__p_0 = ANY (s.""SomeArray"")
LIMIT 2");
}

Expand All @@ -172,7 +172,7 @@ public void Contains_with_column()
AssertSql(
@"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText""
FROM ""SomeEntities"" AS s
WHERE COALESCE(s.""Id"" + 2 = ANY (s.""SomeArray""), FALSE)
WHERE s.""Id"" + 2 = ANY (s.""SomeArray"")
LIMIT 2");
}

Expand Down
7 changes: 6 additions & 1 deletion test/EFCore.PG.FunctionalTests/Query/NullSemanticsQueryNpgsqlTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using Microsoft.EntityFrameworkCore;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.TestModels.NullSemanticsModel;
using Microsoft.EntityFrameworkCore.Query;
using Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure;
using Xunit;

namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query
{
Expand All @@ -12,6 +14,9 @@ public NullSemanticsQueryNpgsqlTest(NullSemanticsQueryNpgsqlFixture fixture)
: base(fixture)
=> Fixture.TestSqlLoggerFactory.Clear();

[ConditionalFact(Skip = "Null semantics for array ANY not yet implemented, #1142")]
public override void Contains_with_local_array_closure_false_with_null() {}

protected override void ClearLog()
=> Fixture.TestSqlLoggerFactory.Clear();

Expand Down
8 changes: 4 additions & 4 deletions test/EFCore.PG.FunctionalTests/Query/SimpleQueryNpgsqlTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,13 @@ public override async Task Contains_with_local_uint_array_closure(bool isAsync)

SELECT e.""EmployeeID"", e.""City"", e.""Country"", e.""FirstName"", e.""ReportsTo"", e.""Title""
FROM ""Employees"" AS e
WHERE COALESCE(e.""EmployeeID"" = ANY (@__ids_0), FALSE)",
WHERE e.""EmployeeID"" = ANY (@__ids_0)",
//
@"@__ids_0='System.Int64[]' (DbType = Object)

SELECT e.""EmployeeID"", e.""City"", e.""Country"", e.""FirstName"", e.""ReportsTo"", e.""Title""
FROM ""Employees"" AS e
WHERE COALESCE(e.""EmployeeID"" = ANY (@__ids_0), FALSE)");
WHERE e.""EmployeeID"" = ANY (@__ids_0)");
}

public override async Task Contains_with_local_nullable_uint_array_closure(bool isAsync)
Expand Down Expand Up @@ -251,7 +251,7 @@ await AssertQuery(

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))");
WHERE c.""Region"" = ANY (@__regions_0) OR ((c.""Region"" IS NULL) AND (array_position(@__regions_0, NULL) IS NOT NULL))");
}

[ConditionalTheory]
Expand All @@ -274,7 +274,7 @@ await AssertQuery(

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))");
WHERE c.""Region"" = ANY (@__regions_0) OR ((c.""Region"" IS NULL) AND (array_position(@__regions_0, NULL) IS NOT NULL))");
}

#endregion Array contains
Expand Down