diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs index 56fcda77c5c..fefe81423f7 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs @@ -550,12 +550,11 @@ protected override ShapedQueryExpression TranslateCast(ShapedQueryExpression sou { var select = (SelectExpression)source.QueryExpression; - // TODO: #34123 - // if ((select.IsDistinct || select.Limit is not null || select.Offset is not null) - // && !TryPushdownIntoSubquery(select)) - // { - // return null; - // } + if ((select.Limit is not null || select.Offset is not null) + && !TryPushdownIntoSubquery(select)) + { + return null; + } select.ApplyDistinct(); @@ -1522,11 +1521,11 @@ private bool TryApplyPredicate(ShapedQueryExpression source, LambdaExpression pr { var select = (SelectExpression)source.QueryExpression; - // TODO: #34123 - // if ((select.Limit is not null || select.Offset is not null) && !TryPushdownIntoSubquery(select)) - // { - // select.PushdownIntoSubquery(); - // } + if ((select.Limit is not null || select.Offset is not null) + && !TryPushdownIntoSubquery(select)) + { + return false; + } if (TranslateLambdaExpression(source, predicate) is SqlExpression translation) { @@ -1543,11 +1542,11 @@ private bool TryApplyPredicate(ShapedQueryExpression source, LambdaExpression pr private bool TryApplyPredicate(SelectExpression select, SqlExpression predicate) { - // TODO: #34123 - // if ((select.Limit is not null || select.Offset is not null) && !TryPushdownIntoSubquery(select, predicate)) - // { - // return false; - // } + if ((select.Limit is not null || select.Offset is not null) + && !TryPushdownIntoSubquery(select)) + { + return false; + } select.ApplyPredicate(predicate); return true; diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/InheritanceQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/InheritanceQueryCosmosTest.cs index 6b468bf6e68..0aa023c39f0 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/InheritanceQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/InheritanceQueryCosmosTest.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.EntityFrameworkCore.Cosmos.Internal; + namespace Microsoft.EntityFrameworkCore.Query; #nullable disable @@ -475,22 +477,9 @@ public override async Task OfType_Union_OfType(bool async) } public override Task Subquery_OfType(bool async) - => Fixture.NoSyncTest( - async, async a => - { - await base.Subquery_OfType(a); - - AssertSql( - """ -@__p_0='5' - -SELECT DISTINCT c -FROM root c -WHERE (c["Discriminator"] IN ("Eagle", "Kiwi") AND (c["Discriminator"] = "Kiwi")) -ORDER BY c["Species"] -OFFSET 0 LIMIT @__p_0 -"""); - }); + => AssertTranslationFailedWithDetails( + () => base.Subquery_OfType(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); public override async Task Union_entity_equality(bool async) { diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs index bb174e94fb5..76ef7f852af 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs @@ -1269,34 +1269,15 @@ await AssertTranslationFailedWithDetails( AssertSql(); } - public override async Task Skip_Distinct(bool async) - { - Assert.Equal( - CosmosStrings.OffsetRequiresLimit, - (await Assert.ThrowsAsync( - () => base.Skip_Distinct(async))).Message); - - AssertSql(); - } + public override Task Skip_Distinct(bool async) + => AssertTranslationFailedWithDetails( + () => base.Skip_Distinct(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); public override Task Skip_Take_Distinct(bool async) - => Fixture.NoSyncTest( - async, async a => - { - await base.Skip_Take_Distinct(a); - - AssertSql( - """ -@__p_0='5' -@__p_1='10' - -SELECT DISTINCT c -FROM root c -WHERE (c["Discriminator"] = "Customer") -ORDER BY c["ContactName"] -OFFSET @__p_0 LIMIT @__p_1 -"""); - }); + => AssertTranslationFailedWithDetails( + () => base.Skip_Take_Distinct(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); public override async Task Skip_Take_Any(bool async) { @@ -1339,54 +1320,15 @@ public override async Task Take_All(bool async) AssertSql(); } - public override async Task Skip_Take_Any_with_predicate(bool async) - { - // Always throws for sync. - if (async) - { - // Top-level parameterless Any(), see #33854. - var exception = await Assert.ThrowsAsync(() => base.Skip_Take_Any_with_predicate(async)); - - Assert.Equal(HttpStatusCode.BadRequest, exception.StatusCode); - - AssertSql( - """ -@__p_0='5' -@__p_1='7' - -SELECT EXISTS ( - SELECT 1 - FROM root c - WHERE ((c["Discriminator"] = "Customer") AND STARTSWITH(c["CustomerID"], "C")) - ORDER BY c["CustomerID"] - OFFSET @__p_0 LIMIT @__p_1) AS c -"""); - } - } - - public override async Task Take_Any_with_predicate(bool async) - { - // Always throws for sync. - if (async) - { - // Top-level parameterless Any(), see #33854. - var exception = await Assert.ThrowsAsync(() => base.Take_Any_with_predicate(async)); - - Assert.Equal(HttpStatusCode.BadRequest, exception.StatusCode); - - AssertSql( - """ -@__p_0='5' + public override Task Skip_Take_Any_with_predicate(bool async) + => AssertTranslationFailedWithDetails( + () => base.Skip_Take_Any_with_predicate(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); -SELECT EXISTS ( - SELECT 1 - FROM root c - WHERE ((c["Discriminator"] = "Customer") AND STARTSWITH(c["CustomerID"], "B")) - ORDER BY c["CustomerID"] - OFFSET 0 LIMIT @__p_0) AS c -"""); - } - } + public override Task Take_Any_with_predicate(bool async) + => AssertTranslationFailedWithDetails( + () => base.Take_Any_with_predicate(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); public override Task OrderBy(bool async) => Fixture.NoSyncTest( @@ -1507,33 +1449,15 @@ public override async Task OrderBy_multiple_queries(bool async) } public override Task Take_Distinct(bool async) - => Fixture.NoSyncTest( - async, async a => - { - await base.Take_Distinct(a); - - AssertSql( - """ -@__p_0='5' - -SELECT DISTINCT c -FROM root c -WHERE (c["Discriminator"] = "Order") -ORDER BY c["OrderID"] -OFFSET 0 LIMIT @__p_0 -"""); - }); + => AssertTranslationFailedWithDetails( + () => base.Take_Distinct(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); - public override async Task Distinct_Take(bool async) - { - // Subquery pushdown. Issue #16156. - await AssertTranslationFailedWithDetails( + public override Task Distinct_Take(bool async) + => AssertTranslationFailedWithDetails( () => base.Distinct_Take(async), CosmosStrings.NoSubqueryPushdown); - AssertSql(); - } - public override async Task Distinct_Take_Count(bool async) { // Cosmos client evaluation. Issue #17246. @@ -2813,73 +2737,20 @@ await AssertTranslationFailedWithDetails( AssertSql(); } - public override async Task OrderBy_skip_take_distinct(bool async) - { - // Always throws for sync. - if (async) - { - // Cosmos client evaluation. Issue #17246. - await Assert.ThrowsAsync( - async () => await base.OrderBy_skip_take_distinct(async)); - - AssertSql( - """ -@__p_0='5' -@__p_1='15' - -SELECT DISTINCT c -FROM root c -WHERE (c["Discriminator"] = "Customer") -ORDER BY c["ContactTitle"], c["ContactName"] -OFFSET @__p_0 LIMIT @__p_1 -"""); - } - } - - public override async Task OrderBy_coalesce_take_distinct(bool async) - { - // Always throws for sync. - if (async) - { - // Cosmos client evaluation. Issue #17246. - await Assert.ThrowsAsync( - async () => await base.OrderBy_coalesce_take_distinct(async)); - - AssertSql( - """ -@__p_0='15' - -SELECT DISTINCT c -FROM root c -WHERE (c["Discriminator"] = "Product") -ORDER BY ((c["UnitPrice"] != null) ? c["UnitPrice"] : 0.0) -OFFSET 0 LIMIT @__p_0 -"""); - } - } - - public override async Task OrderBy_coalesce_skip_take_distinct(bool async) - { - // Always throws for sync. - if (async) - { - // Cosmos client evaluation. Issue #17246. - await Assert.ThrowsAsync( - async () => await base.OrderBy_coalesce_skip_take_distinct(async)); + public override Task OrderBy_skip_take_distinct(bool async) + => AssertTranslationFailedWithDetails( + () => base.OrderBy_skip_take_distinct(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); - AssertSql( - """ -@__p_0='5' -@__p_1='15' + public override Task OrderBy_coalesce_take_distinct(bool async) + => AssertTranslationFailedWithDetails( + () => base.OrderBy_coalesce_take_distinct(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); -SELECT DISTINCT c -FROM root c -WHERE (c["Discriminator"] = "Product") -ORDER BY ((c["UnitPrice"] != null) ? c["UnitPrice"] : 0.0) -OFFSET @__p_0 LIMIT @__p_1 -"""); - } - } + public override Task OrderBy_coalesce_skip_take_distinct(bool async) + => AssertTranslationFailedWithDetails( + () => base.OrderBy_coalesce_skip_take_distinct(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); public override async Task OrderBy_coalesce_skip_take_distinct_take(bool async) { diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSelectQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSelectQueryCosmosTest.cs index dff9c2e0838..9587fe54d8f 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSelectQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSelectQueryCosmosTest.cs @@ -1425,22 +1425,9 @@ await AssertTranslationFailed( } public override Task Projection_take_predicate_projection(bool async) - => Fixture.NoSyncTest( - async, async a => - { - await base.Projection_take_predicate_projection(a); - - AssertSql( - """ -@__p_0='10' - -SELECT ((c["CustomerID"] || " ") || c["City"]) AS Aggregate -FROM root c -WHERE ((c["Discriminator"] = "Customer") AND STARTSWITH(c["CustomerID"], "A")) -ORDER BY c["CustomerID"] -OFFSET 0 LIMIT @__p_0 -"""); - }); + => AssertTranslationFailedWithDetails( + () => base.Projection_take_predicate_projection(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); public override Task Projection_take_projection_doesnt_project_intermittent_column(bool async) => Fixture.NoSyncTest( diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindWhereQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindWhereQueryCosmosTest.cs index 94ae70d42f3..f9fc203eef6 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindWhereQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindWhereQueryCosmosTest.cs @@ -884,29 +884,10 @@ FROM root c """); }); - public override async Task Where_simple_shadow_subquery(bool async) - { - // Always throws for sync. - if (async) - { - await Fixture.NoSyncTest( - async, async a => - { - await Assert.ThrowsAsync(() => base.Where_simple_shadow_subquery(a)); - - AssertSql( - """ -@__p_0='5' - -SELECT c -FROM root c -WHERE ((c["Discriminator"] = "Employee") AND (c["Title"] = "Sales Representative")) -ORDER BY c["EmployeeID"] -OFFSET 0 LIMIT @__p_0 -"""); - }); - } - } + public override Task Where_simple_shadow_subquery(bool async) + => AssertTranslationFailedWithDetails( + () => base.Where_simple_shadow_subquery(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); public override async Task Where_shadow_subquery_FirstOrDefault(bool async) { @@ -1575,21 +1556,9 @@ public override async Task Where_select_many_and(bool async) } public override Task Where_primitive(bool async) - => Fixture.NoSyncTest( - async, async a => - { - await base.Where_primitive(a); - - AssertSql( - """ -@__p_0='9' - -SELECT c["EmployeeID"] -FROM root c -WHERE ((c["Discriminator"] = "Employee") AND (c["EmployeeID"] = 5)) -OFFSET 0 LIMIT @__p_0 -"""); - }); + => AssertTranslationFailedWithDetails( + () => base.Where_primitive(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); public override Task Where_bool_member(bool async) => Fixture.NoSyncTest( @@ -3154,38 +3123,14 @@ FROM root c }); public override Task Where_primitive_tracked(bool async) - => Fixture.NoSyncTest( - async, async a => - { - await base.Where_primitive_tracked(a); - - AssertSql( - """ -@__p_0='9' - -SELECT c -FROM root c -WHERE ((c["Discriminator"] = "Employee") AND (c["EmployeeID"] = 5)) -OFFSET 0 LIMIT @__p_0 -"""); - }); + => AssertTranslationFailedWithDetails( + () => base.Where_primitive_tracked(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); public override Task Where_primitive_tracked2(bool async) - => Fixture.NoSyncTest( - async, async a => - { - await base.Where_primitive_tracked2(a); - - AssertSql( - """ -@__p_0='9' - -SELECT c AS e -FROM root c -WHERE ((c["Discriminator"] = "Employee") AND (c["EmployeeID"] = 5)) -OFFSET 0 LIMIT @__p_0 -"""); - }); + => AssertTranslationFailedWithDetails( + () => base.Where_primitive_tracked2(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); public override Task Where_poco_closure(bool async) => Fixture.NoSyncTest( @@ -3385,6 +3330,25 @@ FROM root c """); }); + #region Evaluation order of predicates + + public override Task Take_and_Where_evaluation_order(bool async) + => AssertTranslationFailedWithDetails( + () => base.Take_and_Where_evaluation_order(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); + + public override Task Skip_and_Where_evaluation_order(bool async) + => AssertTranslationFailedWithDetails( + () => base.Skip_and_Where_evaluation_order(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); + + public override Task Take_and_Distinct_evaluation_order(bool async) + => AssertTranslationFailedWithDetails( + () => base.Take_and_Distinct_evaluation_order(async), + CosmosStrings.LimitOffsetNotSupportedInSubqueries); + + #endregion Evaluation order of predicates + private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected); diff --git a/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs index f8865c6b9ef..8ab230435a2 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs @@ -2544,4 +2544,29 @@ await AssertQuery( ss => ss.Set().Select(x => new DtoWithInterface { Id = x.OrderID }).Where(x => (x as IHaveId).Id == 10252), elementAsserter: (e, a) => AssertEqual(e.Id, a.Id)); } + + #region Evaluation order of predicates + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Take_and_Where_evaluation_order(bool async) + => AssertQuery( + async, + ss => ss.Set().OrderBy(e => e.EmployeeID).Take(3).Where(e => e.EmployeeID % 2 == 0)); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Skip_and_Where_evaluation_order(bool async) + => AssertQuery( + async, + ss => ss.Set().OrderBy(e => e.EmployeeID).Skip(3).Where(e => e.EmployeeID % 2 == 0)); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Take_and_Distinct_evaluation_order(bool async) + => AssertQuery( + async, + ss => ss.Set().Select(c => c.ContactTitle).OrderBy(t => t).Take(3).Distinct()); + + #endregion Evaluation order of predicates } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindWhereQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindWhereQuerySqlServerTest.cs index e913143e580..c40a5fc7c4a 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindWhereQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindWhereQuerySqlServerTest.cs @@ -3425,6 +3425,66 @@ FROM [Orders] AS [o] """); } + #region Evaluation order of predicates + + public override async Task Take_and_Where_evaluation_order(bool async) + { + await base.Take_and_Where_evaluation_order(async); + + AssertSql( + """ +@__p_0='3' + +SELECT [e0].[EmployeeID], [e0].[City], [e0].[Country], [e0].[FirstName], [e0].[ReportsTo], [e0].[Title] +FROM ( + SELECT TOP(@__p_0) [e].[EmployeeID], [e].[City], [e].[Country], [e].[FirstName], [e].[ReportsTo], [e].[Title] + FROM [Employees] AS [e] + ORDER BY [e].[EmployeeID] +) AS [e0] +WHERE [e0].[EmployeeID] % 2 = 0 +ORDER BY [e0].[EmployeeID] +"""); + } + + public override async Task Skip_and_Where_evaluation_order(bool async) + { + await base.Skip_and_Where_evaluation_order(async); + + AssertSql( + """ +@__p_0='3' + +SELECT [e0].[EmployeeID], [e0].[City], [e0].[Country], [e0].[FirstName], [e0].[ReportsTo], [e0].[Title] +FROM ( + SELECT [e].[EmployeeID], [e].[City], [e].[Country], [e].[FirstName], [e].[ReportsTo], [e].[Title] + FROM [Employees] AS [e] + ORDER BY [e].[EmployeeID] + OFFSET @__p_0 ROWS +) AS [e0] +WHERE [e0].[EmployeeID] % 2 = 0 +ORDER BY [e0].[EmployeeID] +"""); + } + + public override async Task Take_and_Distinct_evaluation_order(bool async) + { + await base.Take_and_Distinct_evaluation_order(async); + + AssertSql( + """ +@__p_0='3' + +SELECT DISTINCT [c0].[ContactTitle] +FROM ( + SELECT TOP(@__p_0) [c].[ContactTitle] + FROM [Customers] AS [c] + ORDER BY [c].[ContactTitle] +) AS [c0] +"""); + } + + #endregion Evaluation order of predicates + private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);