Skip to content

Commit

Permalink
Optimize away DISTINCT inside IN/EXISTS/set operations (#34381)
Browse files Browse the repository at this point in the history
Fixes #31016
  • Loading branch information
ranma42 authored Aug 9, 2024
1 parent 802f5f9 commit 915245c
Show file tree
Hide file tree
Showing 11 changed files with 199 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ private static ShapedQueryExpression CreateShapedQueryExpression(IEntityType ent
{
subquery.ClearOrdering();
}
subquery.IsDistinct = false;

translation = _sqlExpressionFactory.Not(_sqlExpressionFactory.Exists(subquery));
subquery = new SelectExpression(translation, _sqlAliasManager);
Expand Down Expand Up @@ -509,6 +510,7 @@ private static ShapedQueryExpression CreateShapedQueryExpression(IEntityType ent
{
subquery.ClearOrdering();
}
subquery.IsDistinct = false;

var translation = _sqlExpressionFactory.Exists(subquery);
var selectExpression = new SelectExpression(translation, _sqlAliasManager);
Expand Down Expand Up @@ -589,6 +591,7 @@ protected override ShapedQueryExpression TranslateConcat(ShapedQueryExpression s
{
subquery.ClearOrdering();
}
subquery.IsDistinct = false;

subquery.ReplaceProjection(new List<Expression> { projection });
subquery.ApplyProjection();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public static SelectExpression CreateImmutable(string alias, List<TableExpressio
/// <summary>
/// A bool value indicating if DISTINCT is applied to projection of this <see cref="SelectExpression" />.
/// </summary>
public bool IsDistinct { get; private set; }
public bool IsDistinct { get; set; }

/// <summary>
/// The list of expressions being projected out from the result set.
Expand Down Expand Up @@ -2006,6 +2006,12 @@ private void ApplySetOperation(
select2.ClearOrdering();
}

if (distinct)
{
select1.IsDistinct = false;
select2.IsDistinct = false;
}

if (_clientProjections.Count > 0
|| select2._clientProjections.Count > 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,30 @@ public override async Task Any_with_multiple_conditions_still_uses_exists(bool a
AssertSql();
}

public override async Task Any_on_distinct(bool async)
{
// Cosmos client evaluation. Issue #17246.
await AssertTranslationFailed(() => base.Any_on_distinct(async));

AssertSql();
}

public override async Task Contains_on_distinct(bool async)
{
// Cosmos client evaluation. Issue #17246.
await AssertTranslationFailed(() => base.Contains_on_distinct(async));

AssertSql();
}

public override async Task All_on_distinct(bool async)
{
// Cosmos client evaluation. Issue #17246.
await AssertTranslationFailed(() => base.All_on_distinct(async));

AssertSql();
}

public override async Task All_top_level(bool async)
{
// Cosmos client evaluation. Issue #17246.
Expand Down Expand Up @@ -3675,7 +3699,7 @@ public override async Task SelectMany_primitive_select_subquery(bool async)
// Cosmos client evaluation. Issue #17246.
Assert.Equal(
CoreStrings.ExpressionParameterizationExceptionSensitive(
"value(Microsoft.EntityFrameworkCore.Query.NorthwindMiscellaneousQueryTestBase`1+<>c__DisplayClass169_0[Microsoft.EntityFrameworkCore.Query.NorthwindQueryCosmosFixture`1[Microsoft.EntityFrameworkCore.TestUtilities.NoopModelCustomizer]]).ss.Set().Any()"),
"value(Microsoft.EntityFrameworkCore.Query.NorthwindMiscellaneousQueryTestBase`1+<>c__DisplayClass172_0[Microsoft.EntityFrameworkCore.Query.NorthwindQueryCosmosFixture`1[Microsoft.EntityFrameworkCore.TestUtilities.NoopModelCustomizer]]).ss.Set().Any()"),
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.SelectMany_primitive_select_subquery(async))).Message);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,27 @@ public virtual Task Any_with_multiple_conditions_still_uses_exists(bool async)
async,
ss => ss.Set<Customer>().Where(c => c.City == "London" && c.Orders.Any(o => o.EmployeeID == 1)));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Any_on_distinct(bool async)
=> AssertQuery(
async,
ss => ss.Set<Customer>().Where(c => c.Orders.Select(o => o.EmployeeID).Distinct().Any(id => id != 1)));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Contains_on_distinct(bool async)
=> AssertQuery(
async,
ss => ss.Set<Customer>().Where(c => c.Orders.Select(o => o.EmployeeID).Distinct().Contains(1u)));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task All_on_distinct(bool async)
=> AssertQuery(
async,
ss => ss.Set<Customer>().Where(c => c.Orders.Select(o => o.EmployeeID).Distinct().All(id => id != 1)));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task All_top_level(bool async)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,48 @@ public virtual Task Select_Except_reference_projection(bool async)
.Where(o => o.CustomerID == "ALFKI")
.Select(o => o.Customer)));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Intersect_on_distinct(bool async)
=> AssertQuery(
async,
ss => ss.Set<Customer>()
.Where(c => c.City == "México D.F.")
.Select(c => c.CompanyName)
.Distinct()
.Intersect(
ss.Set<Customer>()
.Where(s => s.ContactTitle == "Owner")
.Select(c => c.CompanyName)));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Union_on_distinct(bool async)
=> AssertQuery(
async,
ss => ss.Set<Customer>()
.Where(c => c.City == "México D.F.")
.Select(c => c.CompanyName)
.Distinct()
.Union(
ss.Set<Customer>()
.Where(s => s.ContactTitle == "Owner")
.Select(c => c.CompanyName)));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Except_on_distinct(bool async)
=> AssertQuery(
async,
ss => ss.Set<Customer>()
.Where(c => c.City == "México D.F.")
.Select(c => c.CompanyName)
.Distinct()
.Except(
ss.Set<Customer>()
.Where(s => s.ContactTitle == "Owner")
.Select(c => c.CompanyName)));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Include_Union_only_on_one_side_throws(bool async)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2068,7 +2068,7 @@ public override async Task Contains_with_subquery_optional_navigation_scalar_dis
FROM [LevelOne] AS [l]
LEFT JOIN [LevelTwo] AS [l0] ON [l].[Id] = [l0].[Level1_Optional_Id]
WHERE 5 IN (
SELECT DISTINCT CAST(LEN([l1].[Name]) AS int)
SELECT CAST(LEN([l1].[Name]) AS int)
FROM [LevelThree] AS [l1]
WHERE [l0].[Id] IS NOT NULL AND [l0].[Id] = [l1].[OneToMany_Optional_Inverse3Id]
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2068,7 +2068,7 @@ public override async Task Contains_with_subquery_optional_navigation_scalar_dis
FROM [LevelOne] AS [l]
LEFT JOIN [LevelTwo] AS [l0] ON [l].[Id] = [l0].[Level1_Optional_Id]
WHERE 5 IN (
SELECT DISTINCT CAST(LEN([l1].[Name]) AS int)
SELECT CAST(LEN([l1].[Name]) AS int)
FROM [LevelThree] AS [l1]
WHERE [l0].[Id] IS NOT NULL AND [l0].[Id] = [l1].[OneToMany_Optional_Inverse3Id]
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2117,7 +2117,7 @@ FROM [Level1] AS [l0]
WHERE [l0].[OneToOne_Required_PK_Date] IS NOT NULL AND [l0].[Level1_Required_Id] IS NOT NULL AND [l0].[OneToMany_Required_Inverse2Id] IS NOT NULL
) AS [l1] ON [l].[Id] = [l1].[Level1_Optional_Id]
WHERE 5 IN (
SELECT DISTINCT CAST(LEN([l2].[Level3_Name]) AS int)
SELECT CAST(LEN([l2].[Level3_Name]) AS int)
FROM [Level1] AS [l2]
WHERE [l2].[Level2_Required_Id] IS NOT NULL AND [l2].[OneToMany_Required_Inverse3Id] IS NOT NULL AND CASE
WHEN [l1].[OneToOne_Required_PK_Date] IS NOT NULL AND [l1].[Level1_Required_Id] IS NOT NULL AND [l1].[OneToMany_Required_Inverse2Id] IS NOT NULL THEN [l1].[Id]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2119,7 +2119,7 @@ FROM [Level1] AS [l0]
WHERE [l0].[OneToOne_Required_PK_Date] IS NOT NULL AND [l0].[Level1_Required_Id] IS NOT NULL AND [l0].[OneToMany_Required_Inverse2Id] IS NOT NULL
) AS [l1] ON [l].[Id] = [l1].[Level1_Optional_Id]
WHERE 5 IN (
SELECT DISTINCT CAST(LEN([l2].[Level3_Name]) AS int)
SELECT CAST(LEN([l2].[Level3_Name]) AS int)
FROM [Level1] AS [l2]
WHERE [l2].[Level2_Required_Id] IS NOT NULL AND [l2].[OneToMany_Required_Inverse3Id] IS NOT NULL AND CASE
WHEN [l1].[OneToOne_Required_PK_Date] IS NOT NULL AND [l1].[Level1_Required_Id] IS NOT NULL AND [l1].[OneToMany_Required_Inverse2Id] IS NOT NULL THEN [l1].[Id]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1603,6 +1603,52 @@ FROM [Orders] AS [o]
""");
}

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

AssertSql(
"""
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 EXISTS (
SELECT 1
FROM [Orders] AS [o]
WHERE [c].[CustomerID] = [o].[CustomerID] AND ([o].[EmployeeID] <> 1 OR [o].[EmployeeID] IS NULL))
""");
}

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

AssertSql(
"""
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 1 IN (
SELECT [o].[EmployeeID]
FROM [Orders] AS [o]
WHERE [c].[CustomerID] = [o].[CustomerID]
)
""");
}

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

AssertSql(
"""
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 NOT EXISTS (
SELECT 1
FROM [Orders] AS [o]
WHERE [c].[CustomerID] = [o].[CustomerID] AND [o].[EmployeeID] = 1)
""");
}

public override async Task All_top_level(bool async)
{
await base.All_top_level(async);
Expand Down Expand Up @@ -4061,7 +4107,7 @@ WHERE EXISTS (
SELECT 1
FROM [Customers] AS [c1]
WHERE EXISTS (
SELECT DISTINCT 1
SELECT 1
FROM (
SELECT TOP(10) 1 AS empty
FROM [Customers] AS [c2]
Expand Down Expand Up @@ -7263,7 +7309,7 @@ FROM OPENJSON(@__ids_0) WITH ([value] int '$') AS [i0]

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

AssertSql(
"""
Expand All @@ -7280,7 +7326,7 @@ FROM OPENJSON(@__data_0) WITH ([value] nvarchar(45) '$') AS [d]

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

AssertSql(
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1370,6 +1370,54 @@ public override async Task Collection_projection_before_set_operation_fails(bool
AssertSql();
}

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

AssertSql(
"""
SELECT [c].[CompanyName]
FROM [Customers] AS [c]
WHERE [c].[City] = N'México D.F.'
INTERSECT
SELECT [c0].[CompanyName]
FROM [Customers] AS [c0]
WHERE [c0].[ContactTitle] = N'Owner'
""");
}

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

AssertSql(
"""
SELECT [c].[CompanyName]
FROM [Customers] AS [c]
WHERE [c].[City] = N'México D.F.'
UNION
SELECT [c0].[CompanyName]
FROM [Customers] AS [c0]
WHERE [c0].[ContactTitle] = N'Owner'
""");
}

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

AssertSql(
"""
SELECT [c].[CompanyName]
FROM [Customers] AS [c]
WHERE [c].[City] = N'México D.F.'
EXCEPT
SELECT [c0].[CompanyName]
FROM [Customers] AS [c0]
WHERE [c0].[ContactTitle] = N'Owner'
""");
}

public override async Task Include_Union_only_on_one_side_throws(bool async)
{
await base.Include_Union_only_on_one_side_throws(async);
Expand Down

0 comments on commit 915245c

Please sign in to comment.