Skip to content

Commit 16154d5

Browse files
rojiroji
committed
Keep InExpression even when complex compensation is required
Except for SQL Server
1 parent 8cdef22 commit 16154d5

File tree

6 files changed

+362
-42
lines changed

6 files changed

+362
-42
lines changed

‎src/EFCore.Relational/Query/SqlNullabilityProcessor.cs‎

Lines changed: 85 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -672,6 +672,8 @@ protected virtual SqlExpression VisitIn(InExpression inExpression, bool allowOpt
672672
// we want to avoid double visitation
673673
var subqueryProjection = subquery.Projection.Single().Expression;
674674

675+
inExpression = inExpression.Update(item, values: null, subquery);
676+
675677
var unwrappedSubqueryProjection = subqueryProjection;
676678
while (unwrappedSubqueryProjection is SqlUnaryExpression
677679
{
@@ -688,48 +690,90 @@ protected virtual SqlExpression VisitIn(InExpression inExpression, bool allowOpt
688690
{
689691
nullable = itemNullable || projectionNullable;
690692

691-
return inExpression.Update(item, values: null, subquery);
693+
return inExpression;
692694
}
693695

694696
nullable = false;
695697

696-
// Null item and non-nullable projection - return false immediately
697-
if (IsNull(item) && !projectionNullable)
698-
{
699-
return _sqlExpressionFactory.Constant(false, inExpression.TypeMapping);
700-
}
698+
// SQL IN returns null when the item is null, and when the values (subquery projection) contains NULL and no match was made.
701699

702-
// SQL IN returns null when the item is null, and when the values contains NULL and no match was made.
703-
// If both sides are non-nullable, IN never returns null, so is safe to use as-is.
704-
// If one side is null but not the other, IN returns null when the item isn't found; this is also safe to use as-is in
705-
// optimized expansion (null interpreted as false).
706-
// Note that we could coalesce NULL to false when we're not in optimized expansion, but that leads to more complex SQL that may
707-
// be worse performance-wise than the EXISTS rewrite below.
708-
if (!itemNullable && !projectionNullable
709-
|| allowOptimizedExpansion && (itemNullable ^ projectionNullable))
700+
switch ((itemNullable, projectionNullable))
710701
{
711-
return inExpression.Update(item, values: null, subquery);
712-
}
702+
// If both sides are non-nullable, IN never returns null, so is safe to use as-is.
703+
case (false, false):
704+
return inExpression;
705+
706+
case (true, false):
707+
{
708+
// If the item is actually null (not just nullable) and the projection is non-nullable, just return false immediately:
709+
// WHERE NULL IN (SELECT NonNullable FROM foo) -> false
710+
if (IsNull(item))
711+
{
712+
return _sqlExpressionFactory.Constant(false, inExpression.TypeMapping);
713+
}
714+
715+
// Otherwise, since the projection is non-nullable, NULL will only be returned if the item wasn't found. Use as-is
716+
// in optimized expansion (NULL is interpreted as false anyway), or compensate for the item being possibly null:
717+
// WHERE Nullable IN (SELECT NonNullable FROM foo) -> WHERE Nullable IN (SELECT NonNullable FROM foo) AND Nullable IS NOT NULL
718+
// WHERE Nullable NOT IN (SELECT NonNullable FROM foo) -> WHERE Nullable NOT IN (SELECT NonNullable FROM foo) OR Nullable IS NULL
719+
return allowOptimizedExpansion
720+
? inExpression
721+
: inExpression.IsNegated
722+
? _sqlExpressionFactory.OrElse(inExpression, _sqlExpressionFactory.IsNull(item))
723+
: _sqlExpressionFactory.AndAlso(inExpression, _sqlExpressionFactory.IsNotNull(item));
724+
}
713725

714-
// We'll need to mutate the subquery to introduce the predicate inside it, but it might be referenced by other places in the
715-
// tree, so we create a copy to work on.
716-
717-
// No need for a projection with EXISTS, clear it to get SELECT 1
718-
subquery = subquery.Update(
719-
Array.Empty<ProjectionExpression>(),
720-
subquery.Tables,
721-
subquery.Predicate,
722-
subquery.GroupBy,
723-
subquery.Having,
724-
subquery.Orderings,
725-
subquery.Limit,
726-
subquery.Offset);
727-
728-
var predicate = VisitSqlBinary(_sqlExpressionFactory.Equal(subqueryProjection, item), allowOptimizedExpansion: true, out _);
729-
subquery.ApplyPredicate(predicate);
730-
subquery.ClearOrdering();
731-
732-
return _sqlExpressionFactory.Exists(subquery, inExpression.IsNegated);
726+
case (false, true):
727+
{
728+
// If the item is non-nullable but the projection is nullable, NULL will only be returned if the item wasn't found
729+
// (as with the above case).
730+
// Use as-is in optimized expansion (NULL is interpreted as false anyway), or compensate by coalescing NULL to false:
731+
// WHERE NonNullable IN (SELECT Nullable FROM foo) -> WHERE COALESCE(NonNullable IN (SELECT Nullable FROM foo), false)
732+
if (allowOptimizedExpansion)
733+
{
734+
return inExpression;
735+
}
736+
737+
// On SQL Server, EXISTS isn't less efficient than IN, and the additional COALESCE (and CASE/WHEN which it requires)
738+
// add unneeded clutter (and possibly hurt perf). So allow providers to prefer EXISTS.
739+
if (PreferExistsToComplexIn)
740+
{
741+
goto TransformToExists;
742+
}
743+
744+
return inExpression.IsNegated
745+
? _sqlExpressionFactory.Not(
746+
_sqlExpressionFactory.Coalesce(inExpression.Negate(), _sqlExpressionFactory.Constant(false)))
747+
: _sqlExpressionFactory.Coalesce(inExpression, _sqlExpressionFactory.Constant(false));
748+
}
749+
750+
case (true, true):
751+
TransformToExists:
752+
// Worst case: both sides are nullable; there's no way to distinguish the item was found or not.
753+
// We rewrite to an EXISTS subquery where we can generate a precise predicate to check for what we need. Note that this
754+
// performs (significantly) worse than an IN expression, since it involves a correlated subquery.
755+
756+
// We'll need to mutate the subquery to introduce the predicate inside it, but it might be referenced by other places in
757+
// the tree, so we create a copy to work on.
758+
759+
// No need for a projection with EXISTS, clear it to get SELECT 1
760+
subquery = subquery.Update(
761+
Array.Empty<ProjectionExpression>(),
762+
subquery.Tables,
763+
subquery.Predicate,
764+
subquery.GroupBy,
765+
subquery.Having,
766+
subquery.Orderings,
767+
subquery.Limit,
768+
subquery.Offset);
769+
770+
var predicate = VisitSqlBinary(
771+
_sqlExpressionFactory.Equal(subqueryProjection, item), allowOptimizedExpansion: true, out _);
772+
subquery.ApplyPredicate(predicate);
773+
subquery.ClearOrdering();
774+
775+
return _sqlExpressionFactory.Exists(subquery, inExpression.IsNegated);
776+
}
733777
}
734778

735779
// Non-subquery case
@@ -1303,6 +1347,12 @@ protected virtual SqlExpression VisitJsonScalar(
13031347
return jsonScalarExpression;
13041348
}
13051349

1350+
/// <summary>
1351+
/// Determines whether an <see cref="InExpression" /> will be transformed to an <see cref="ExistsExpression" /> when it would
1352+
/// otherwise require complex compensation for null semantics.
1353+
/// </summary>
1354+
protected virtual bool PreferExistsToComplexIn => false;
1355+
13061356
private static bool? TryGetBoolConstantValue(SqlExpression? expression)
13071357
=> expression is SqlConstantExpression { Value: bool boolValue } ? boolValue : null;
13081358

‎src/EFCore.SqlServer/Query/Internal/SqlServerSqlNullabilityProcessor.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,4 +104,12 @@ protected virtual SqlExpression VisitSqlServerAggregateFunction(
104104
orderings ?? aggregateFunctionExpression.Orderings)
105105
: aggregateFunctionExpression;
106106
}
107+
108+
/// <summary>
109+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
110+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
111+
/// any release. You should only use it directly in your code with extreme caution and knowing that
112+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
113+
/// </summary>
114+
protected override bool PreferExistsToComplexIn => true;
107115
}

‎test/EFCore.Relational.Specification.Tests/Query/NullSemanticsQueryTestBase.cs‎

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1154,7 +1154,75 @@ await AssertQueryScalar(
11541154

11551155
[ConditionalTheory]
11561156
[MemberData(nameof(IsAsyncData))]
1157-
public virtual async Task Null_semantics_contains_non_nullable_argument(bool async)
1157+
public virtual async Task Null_semantics_contains_non_nullable_item_with_non_nullable_subquery(bool async)
1158+
{
1159+
await AssertQueryScalar(
1160+
async,
1161+
ss => ss.Set<NullSemanticsEntity1>()
1162+
.Where(e => ss.Set<NullSemanticsEntity2>().Select(e => e.StringA).Contains(e.StringA))
1163+
.Select(e => e.Id));
1164+
1165+
await AssertQueryScalar(
1166+
async,
1167+
ss => ss.Set<NullSemanticsEntity1>()
1168+
.Where(e => !ss.Set<NullSemanticsEntity2>().Select(e => e.StringA).Contains(e.StringA))
1169+
.Select(e => e.Id));
1170+
}
1171+
1172+
[ConditionalTheory]
1173+
[MemberData(nameof(IsAsyncData))]
1174+
public virtual async Task Null_semantics_contains_nullable_item_with_non_nullable_subquery(bool async)
1175+
{
1176+
await AssertQueryScalar(
1177+
async,
1178+
ss => ss.Set<NullSemanticsEntity1>()
1179+
.Where(e => ss.Set<NullSemanticsEntity2>().Select(e => e.StringA).Contains(e.NullableStringA))
1180+
.Select(e => e.Id));
1181+
1182+
await AssertQueryScalar(
1183+
async,
1184+
ss => ss.Set<NullSemanticsEntity1>()
1185+
.Where(e => !ss.Set<NullSemanticsEntity2>().Select(e => e.StringA).Contains(e.NullableStringA))
1186+
.Select(e => e.Id));
1187+
}
1188+
1189+
[ConditionalTheory]
1190+
[MemberData(nameof(IsAsyncData))]
1191+
public virtual async Task Null_semantics_contains_non_nullable_item_with_nullable_subquery(bool async)
1192+
{
1193+
await AssertQueryScalar(
1194+
async,
1195+
ss => ss.Set<NullSemanticsEntity1>()
1196+
.Where(e => ss.Set<NullSemanticsEntity2>().Select(e => e.NullableStringA).Contains(e.StringA))
1197+
.Select(e => e.Id));
1198+
1199+
await AssertQueryScalar(
1200+
async,
1201+
ss => ss.Set<NullSemanticsEntity1>()
1202+
.Where(e => !ss.Set<NullSemanticsEntity2>().Select(e => e.NullableStringA).Contains(e.StringA))
1203+
.Select(e => e.Id));
1204+
}
1205+
1206+
[ConditionalTheory]
1207+
[MemberData(nameof(IsAsyncData))]
1208+
public virtual async Task Null_semantics_contains_nullable_item_with_nullable_subquery(bool async)
1209+
{
1210+
await AssertQueryScalar(
1211+
async,
1212+
ss => ss.Set<NullSemanticsEntity1>()
1213+
.Where(e => ss.Set<NullSemanticsEntity2>().Select(e => e.NullableStringA).Contains(e.NullableStringA))
1214+
.Select(e => e.Id));
1215+
1216+
await AssertQueryScalar(
1217+
async,
1218+
ss => ss.Set<NullSemanticsEntity1>()
1219+
.Where(e => !ss.Set<NullSemanticsEntity2>().Select(e => e.NullableStringA).Contains(e.NullableStringA))
1220+
.Select(e => e.Id));
1221+
}
1222+
1223+
[ConditionalTheory]
1224+
[MemberData(nameof(IsAsyncData))]
1225+
public virtual async Task Null_semantics_contains_non_nullable_item_with_values(bool async)
11581226
{
11591227
var ids = new List<int?>
11601228
{

‎test/EFCore.SqlServer.FunctionalTests/Query/NullSemanticsQuerySqlServerTest.cs‎

Lines changed: 98 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1921,9 +1921,105 @@ WHERE [e].[NullableIntA] IS NOT NULL
19211921
""");
19221922
}
19231923

1924-
public override async Task Null_semantics_contains_non_nullable_argument(bool async)
1924+
public override async Task Null_semantics_contains_non_nullable_item_with_non_nullable_subquery(bool async)
19251925
{
1926-
await base.Null_semantics_contains_non_nullable_argument(async);
1926+
await base.Null_semantics_contains_non_nullable_item_with_non_nullable_subquery(async);
1927+
1928+
AssertSql(
1929+
"""
1930+
SELECT [e].[Id]
1931+
FROM [Entities1] AS [e]
1932+
WHERE [e].[StringA] IN (
1933+
SELECT [e0].[StringA]
1934+
FROM [Entities2] AS [e0]
1935+
)
1936+
""",
1937+
//
1938+
"""
1939+
SELECT [e].[Id]
1940+
FROM [Entities1] AS [e]
1941+
WHERE [e].[StringA] NOT IN (
1942+
SELECT [e0].[StringA]
1943+
FROM [Entities2] AS [e0]
1944+
)
1945+
""");
1946+
}
1947+
1948+
public override async Task Null_semantics_contains_nullable_item_with_non_nullable_subquery(bool async)
1949+
{
1950+
await base.Null_semantics_contains_nullable_item_with_non_nullable_subquery(async);
1951+
1952+
AssertSql(
1953+
"""
1954+
SELECT [e].[Id]
1955+
FROM [Entities1] AS [e]
1956+
WHERE [e].[NullableStringA] IN (
1957+
SELECT [e0].[StringA]
1958+
FROM [Entities2] AS [e0]
1959+
)
1960+
""",
1961+
//
1962+
"""
1963+
SELECT [e].[Id]
1964+
FROM [Entities1] AS [e]
1965+
WHERE [e].[NullableStringA] NOT IN (
1966+
SELECT [e0].[StringA]
1967+
FROM [Entities2] AS [e0]
1968+
) OR [e].[NullableStringA] IS NULL
1969+
""");
1970+
}
1971+
1972+
public override async Task Null_semantics_contains_non_nullable_item_with_nullable_subquery(bool async)
1973+
{
1974+
await base.Null_semantics_contains_non_nullable_item_with_nullable_subquery(async);
1975+
1976+
AssertSql(
1977+
"""
1978+
SELECT [e].[Id]
1979+
FROM [Entities1] AS [e]
1980+
WHERE [e].[StringA] IN (
1981+
SELECT [e0].[NullableStringA]
1982+
FROM [Entities2] AS [e0]
1983+
)
1984+
""",
1985+
//
1986+
"""
1987+
SELECT [e].[Id]
1988+
FROM [Entities1] AS [e]
1989+
WHERE NOT EXISTS (
1990+
SELECT 1
1991+
FROM [Entities2] AS [e0]
1992+
WHERE [e0].[NullableStringA] = [e].[StringA])
1993+
""");
1994+
}
1995+
1996+
public override async Task Null_semantics_contains_nullable_item_with_nullable_subquery(bool async)
1997+
{
1998+
await base.Null_semantics_contains_nullable_item_with_nullable_subquery(async);
1999+
2000+
AssertSql(
2001+
"""
2002+
SELECT [e].[Id]
2003+
FROM [Entities1] AS [e]
2004+
WHERE EXISTS (
2005+
SELECT 1
2006+
FROM [Entities2] AS [e0]
2007+
WHERE [e0].[NullableStringA] = [e].[NullableStringA] OR ([e0].[NullableStringA] IS NULL AND [e].[NullableStringA] IS NULL))
2008+
""",
2009+
//
2010+
"""
2011+
SELECT [e].[Id]
2012+
FROM [Entities1] AS [e]
2013+
WHERE NOT EXISTS (
2014+
SELECT 1
2015+
FROM [Entities2] AS [e0]
2016+
WHERE [e0].[NullableStringA] = [e].[NullableStringA] OR ([e0].[NullableStringA] IS NULL AND [e].[NullableStringA] IS NULL))
2017+
""");
2018+
}
2019+
2020+
public override async Task Null_semantics_contains_non_nullable_item_with_values(bool async)
2021+
{
2022+
await base.Null_semantics_contains_non_nullable_item_with_values(async);
19272023

19282024
AssertSql(
19292025
"""

‎test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1904,10 +1904,10 @@ public override async Task Correlated_collection_with_complex_order_by_funcletiz
19041904
SELECT "g"."Nickname", "g"."SquadId", "w"."Name", "w"."Id"
19051905
FROM "Gears" AS "g"
19061906
LEFT JOIN "Weapons" AS "w" ON "g"."FullName" = "w"."OwnerFullName"
1907-
ORDER BY EXISTS (
1908-
SELECT 1
1907+
ORDER BY COALESCE("g"."Nickname" IN (
1908+
SELECT "n"."value"
19091909
FROM json_each(@__nicknames_0) AS "n"
1910-
WHERE "n"."value" = "g"."Nickname") DESC, "g"."Nickname", "g"."SquadId"
1910+
), 0) DESC, "g"."Nickname", "g"."SquadId"
19111911
""");
19121912
}
19131913

0 commit comments

Comments
 (0)