From a594e2f81f02b9bd435c316c7095362738ef965f Mon Sep 17 00:00:00 2001 From: Christopher Jolly Date: Mon, 25 Sep 2023 14:22:08 +0800 Subject: [PATCH] DISTINCT AND TOP can't be in the same stateement with Jet. Push DISTINCT into a subquery --- ...yableMethodTranslatingExpressionVisitor.cs | 7 + .../NorthwindMiscellaneousQueryJetTest.cs | 37 ++-- .../Query/QueryLoggingJetTest.cs | 168 ++++++++++-------- test/EFCore.Jet.FunctionalTests/config.json | 8 +- 4 files changed, 130 insertions(+), 90 deletions(-) diff --git a/src/EFCore.Jet/Query/Internal/JetQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Jet/Query/Internal/JetQueryableMethodTranslatingExpressionVisitor.cs index 07cfbb2..13af1c8 100644 --- a/src/EFCore.Jet/Query/Internal/JetQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Jet/Query/Internal/JetQueryableMethodTranslatingExpressionVisitor.cs @@ -150,6 +150,13 @@ public class JetQueryableMethodTranslatingExpressionVisitor : RelationalQueryabl }*/ } + + //With Jet DISTINCT and TOP can't be used together in the same statement + //Make the DISTINCT into a subquery + if (selectExpression.IsDistinct) + { + selectExpression.PushdownIntoSubquery(); + } return base.TranslateTake(source, count); } diff --git a/test/EFCore.Jet.FunctionalTests/Query/NorthwindMiscellaneousQueryJetTest.cs b/test/EFCore.Jet.FunctionalTests/Query/NorthwindMiscellaneousQueryJetTest.cs index 631a1dc..972a9fb 100644 --- a/test/EFCore.Jet.FunctionalTests/Query/NorthwindMiscellaneousQueryJetTest.cs +++ b/test/EFCore.Jet.FunctionalTests/Query/NorthwindMiscellaneousQueryJetTest.cs @@ -2126,13 +2126,16 @@ ORDER BY `t`.`OrderID`"); await base.Distinct_Take_Count(isAsync); AssertSql( - $@"{AssertSqlHelper.Declaration("@__p_0='5'")} - + """ SELECT COUNT(*) FROM ( - SELECT DISTINCT TOP {AssertSqlHelper.Parameter("@__p_0")} `o`.`OrderID`, `o`.`CustomerID`, `o`.`EmployeeID`, `o`.`OrderDate` - FROM `Orders` AS `o` -) AS `t`"); + SELECT TOP 5 `t`.`OrderID` + FROM ( + SELECT DISTINCT `o`.`OrderID`, `o`.`CustomerID`, `o`.`EmployeeID`, `o`.`OrderDate` + FROM `Orders` AS `o` + ) AS `t` +) AS `t0` +"""); } public override async Task OrderBy_shadow(bool isAsync) @@ -3348,17 +3351,21 @@ FROM ( await base.OrderBy_coalesce_skip_take_distinct_take(isAsync); AssertSql( - $@"{AssertSqlHelper.Declaration("@__p_0='5'")} - -{AssertSqlHelper.Declaration("@__p_1='15'")} - -SELECT DISTINCT TOP {AssertSqlHelper.Parameter("@__p_0")} `t`.`ProductID`, `t`.`Discontinued`, `t`.`ProductName`, `t`.`SupplierID`, `t`.`UnitPrice`, `t`.`UnitsInStock` + """ +SELECT TOP 5 `t1`.`ProductID`, `t1`.`Discontinued`, `t1`.`ProductName`, `t1`.`SupplierID`, `t1`.`UnitPrice`, `t1`.`UnitsInStock` FROM ( - SELECT `p`.`ProductID`, `p`.`Discontinued`, `p`.`ProductName`, `p`.`SupplierID`, `p`.`UnitPrice`, `p`.`UnitsInStock`, IIF(`p`.`UnitPrice` IS NULL, NULL, `p`.`UnitPrice`) AS `c` - FROM `Products` AS `p` - ORDER BY IIF(`p`.`UnitPrice` IS NULL, NULL, `p`.`UnitPrice`) - SKIP {AssertSqlHelper.Parameter("@__p_0")} FETCH NEXT {AssertSqlHelper.Parameter("@__p_1")} ROWS ONLY -) AS `t`"); + SELECT DISTINCT `t0`.`ProductID`, `t0`.`Discontinued`, `t0`.`ProductName`, `t0`.`SupplierID`, `t0`.`UnitPrice`, `t0`.`UnitsInStock` + FROM ( + SELECT TOP 15 `t`.`ProductID`, `t`.`Discontinued`, `t`.`ProductName`, `t`.`SupplierID`, `t`.`UnitPrice`, `t`.`UnitsInStock` + FROM ( + SELECT TOP 20 `p`.`ProductID`, `p`.`Discontinued`, `p`.`ProductName`, `p`.`SupplierID`, `p`.`UnitPrice`, `p`.`UnitsInStock`, IIF(`p`.`UnitPrice` IS NULL, 0.0, `p`.`UnitPrice`) AS `c` + FROM `Products` AS `p` + ORDER BY IIF(`p`.`UnitPrice` IS NULL, 0.0, `p`.`UnitPrice`) + ) AS `t` + ORDER BY `t`.`c` DESC + ) AS `t0` +) AS `t1` +"""); } public override async Task OrderBy_skip_take_distinct_orderby_take(bool isAsync) diff --git a/test/EFCore.Jet.FunctionalTests/Query/QueryLoggingJetTest.cs b/test/EFCore.Jet.FunctionalTests/Query/QueryLoggingJetTest.cs index e8bd8b4..fac50be 100644 --- a/test/EFCore.Jet.FunctionalTests/Query/QueryLoggingJetTest.cs +++ b/test/EFCore.Jet.FunctionalTests/Query/QueryLoggingJetTest.cs @@ -31,17 +31,33 @@ namespace EntityFrameworkCore.Jet.FunctionalTests.Query [ConditionalFact] public virtual void Queryable_simple() { - using (var context = CreateContext()) - { - var customers - = context.Set() - .ToList(); + using var context = CreateContext(); + var customers + = context.Set() + .ToList(); - Assert.NotNull(customers); - Assert.StartsWith( - "queryContext => new QueryingEnumerable(", - Fixture.TestSqlLoggerFactory.Log[0].Message); - } + Assert.NotNull(customers); + + Assert.StartsWith( + "Compiling query expression: ", + Fixture.TestSqlLoggerFactory.Log[0].Message); + Assert.StartsWith( + "Generated query execution expression: " + Environment.NewLine + "'queryContext => new SingleQueryingEnumerable(", + Fixture.TestSqlLoggerFactory.Log[1].Message); + } + + [ConditionalFact] + public virtual void Queryable_simple_split() + { + using var context = CreateContext(); + var customers + = context.Set().AsSplitQuery() + .ToList(); + + Assert.NotNull(customers); + Assert.StartsWith( + "Generated query execution expression: " + Environment.NewLine + "'queryContext => new SplitQueryingEnumerable(", + Fixture.TestSqlLoggerFactory.Log[1].Message); } [ConditionalFact] @@ -66,78 +82,88 @@ namespace EntityFrameworkCore.Jet.FunctionalTests.Query } } - [ConditionalFact(Skip = "Issue#17498")] - public virtual void Query_with_ignored_include_should_log_warning() + [ConditionalFact] + public virtual void Include_navigation() { - using (var context = CreateContext()) - { - var customers - = context.Customers - .Include(c => c.Orders) - .Select(c => c.CustomerID) - .ToList(); + using var context = CreateContext(); + var customers + = context.Set() + .Where(c => c.CustomerID == "ALFKI") + .Include(c => c.Orders) + .ToList(); - Assert.NotNull(customers); - Assert.Contains( -#pragma warning disable CS0612 // Type or member is obsolete - CoreResources.LogNavigationBaseIncludeIgnored(new TestLogger()).GenerateMessage("`c`.Orders"), - Fixture.TestSqlLoggerFactory.Log.Select(l => l.Message)); -#pragma warning restore CS0612 // Type or member is obsolete - } + Assert.NotNull(customers); + + Assert.Equal( + "Including navigation: 'Customer.Orders'.", + Fixture.TestSqlLoggerFactory.Log[1].Message); } - [ConditionalFact(Skip = "Issue#17498")] - public virtual void Include_navigation() + [ConditionalFact] + public virtual void Skip_without_order_by() { - using (var context = CreateContext()) - { - var customers - = context.Set() - .Include(c => c.Orders) - .ToList(); + using var context = CreateContext(); + var customers = context.Set().Skip(85).ToList(); - Assert.NotNull(customers); + Assert.NotNull(customers); - Assert.Equal( - "Compiling query model: " + _eol + "'(from Customer c in DbSet" + _eol + @"select `c`).Include(""Orders"")'" - , - Fixture.TestSqlLoggerFactory.Log[0].Message); - Assert.Equal( - "Including navigation: '`c`.Orders'" - , - Fixture.TestSqlLoggerFactory.Log[1].Message); - Assert.StartsWith( - "Optimized query model: " - + _eol - + "'from Customer c in DbSet" - + _eol - + @"order by EF.Property(?`c`?, ""CustomerID"") asc" - + _eol - + "select Customer _Include(" - , - Fixture.TestSqlLoggerFactory.Log[2].Message); - } + Assert.Equal( + CoreResources.LogRowLimitingOperationWithoutOrderBy(new TestLogger()).GenerateMessage(), + Fixture.TestSqlLoggerFactory.Log[1].Message); } - [ConditionalFact(Skip = "Issue #16752")] - public virtual void GroupBy_Include_collection_ignored() + [ConditionalFact] + public virtual void Take_without_order_by() { - using (var context = CreateContext()) - { - var orders = context.Orders - .GroupBy(o => o.OrderID) - .Select(g => g.OrderBy(o => o.OrderID).FirstOrDefault()) - .Include(o => o.OrderDetails) - .ToList(); + using var context = CreateContext(); + var customers = context.Set().Take(5).ToList(); - Assert.NotNull(orders); - Assert.Contains( -#pragma warning disable CS0612 // Type or member is obsolete - CoreResources.LogNavigationBaseIncludeIgnored(new TestLogger()).GenerateMessage( -#pragma warning restore CS0612 // Type or member is obsolete - "{from Order o in `g` orderby `o`.OrderID asc select `o` => FirstOrDefault()}.OrderDetails"), - Fixture.TestSqlLoggerFactory.Log.Select(l => l.Message)); - } + Assert.NotNull(customers); + + Assert.Equal( + CoreResources.LogRowLimitingOperationWithoutOrderBy(new TestLogger()).GenerateMessage(), + Fixture.TestSqlLoggerFactory.Log[1].Message); + } + + [ConditionalFact] + public virtual void FirstOrDefault_without_filter_order_by() + { + using var context = CreateContext(); + var customer = context.Set().FirstOrDefault(); + + Assert.NotNull(customer); + + Assert.Equal( + CoreResources.LogFirstWithoutOrderByAndFilter(new TestLogger()).GenerateMessage(), + Fixture.TestSqlLoggerFactory.Log[1].Message); + } + + [ConditionalFact] + public virtual void Distinct_used_after_order_by() + { + using var context = CreateContext(); + var customers = context.Set().OrderBy(x => x.Address).Distinct().Take(5).ToList(); + + Assert.NotEmpty(customers); + + Assert.Equal( + CoreResources.LogDistinctAfterOrderByWithoutRowLimitingOperatorWarning(new TestLogger()) + .GenerateMessage(), + Fixture.TestSqlLoggerFactory.Log[1].Message); + } + + [ConditionalFact] + public virtual void Include_collection_does_not_generate_warning() + { + using var context = CreateContext(); + var customer = context.Set().Include(e => e.Orders).AsSplitQuery().Single(e => e.CustomerID == "ALFKI"); + + Assert.NotNull(customer); + Assert.Equal(6, customer.Orders.Count); + + Assert.DoesNotContain( + CoreResources.LogRowLimitingOperationWithoutOrderBy(new TestLogger()).GenerateMessage(), + Fixture.TestSqlLoggerFactory.Log.Select(e => e.Message)); } [ConditionalFact] diff --git a/test/EFCore.Jet.FunctionalTests/config.json b/test/EFCore.Jet.FunctionalTests/config.json index 6c66070..d97040e 100644 --- a/test/EFCore.Jet.FunctionalTests/config.json +++ b/test/EFCore.Jet.FunctionalTests/config.json @@ -1,7 +1,7 @@ { - "Test": { - "Jet": { - "DefaultConnection": "DBQ=Jet.accdb" - } + "Test": { + "Jet": { + "DefaultConnection": "Provider=Microsoft.ACE.OLEDB.12.0;Data Source=Jet.accdb;Persist Security Info=False;" } + } }