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.

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

Optimize away DISTINCT inside IN/EXISTS/set operations #34381

Merged
merged 4 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 @@ -953,6 +953,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 @@ -366,6 +366,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