From 81886272a761df8fafe4970b895b1e1fe35effb8 Mon Sep 17 00:00:00 2001 From: Maurycy Markowski Date: Thu, 26 Jan 2023 10:39:03 -0800 Subject: [PATCH] Fix to #29667 - Count after Take throws "No column name was specified for column 1 of 't'." (#30134) Problem was that in some cases (e.g. count over keyless entity that has been pushed down) we now generate empty projection in the subquery, where before we were projecting some columns. SQL Server doesn't allow unaliased projection in the subquery, so the fix is to simply add an alias to the empty projection that we generate. Fixes #29667 --- .../Query/QuerySqlGenerator.cs | 11 +++- .../Internal/SqlServerQuerySqlGenerator.cs | 15 ++++++ ...NorthwindKeylessEntitiesQueryCosmosTest.cs | 24 +++++++++ .../NorthwindKeylessEntitiesQueryTestBase.cs | 21 ++++++++ ...thwindKeylessEntitiesQuerySqlServerTest.cs | 50 +++++++++++++++++++ 5 files changed, 120 insertions(+), 1 deletion(-) diff --git a/src/EFCore.Relational/Query/QuerySqlGenerator.cs b/src/EFCore.Relational/Query/QuerySqlGenerator.cs index 7dd6cbf143d..7e25529c7a6 100644 --- a/src/EFCore.Relational/Query/QuerySqlGenerator.cs +++ b/src/EFCore.Relational/Query/QuerySqlGenerator.cs @@ -247,7 +247,7 @@ protected override Expression VisitSelect(SelectExpression selectExpression) } else { - _relationalCommandBuilder.Append("1"); + GenerateEmptyProjection(selectExpression); } if (selectExpression.Tables.Any()) @@ -306,6 +306,15 @@ protected virtual void GeneratePseudoFromClause() { } + /// + /// Generates empty projection for a SelectExpression. + /// + /// SelectExpression for which the empty projection will be generated. + protected virtual void GenerateEmptyProjection(SelectExpression selectExpression) + { + _relationalCommandBuilder.Append("1"); + } + /// protected override Expression VisitProjection(ProjectionExpression projectionExpression) { diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs index ec08ad38f62..bc4285b3116 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs @@ -124,6 +124,21 @@ protected override Expression VisitUpdate(UpdateExpression updateExpression) RelationalStrings.ExecuteOperationWithUnsupportedOperatorInSqlGeneration(nameof(RelationalQueryableExtensions.ExecuteUpdate))); } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + protected override void GenerateEmptyProjection(SelectExpression selectExpression) + { + base.GenerateEmptyProjection(selectExpression); + if (selectExpression.Alias != null) + { + Sql.Append(" AS empty"); + } + } + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindKeylessEntitiesQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindKeylessEntitiesQueryCosmosTest.cs index efda76c9a06..52bf66aa0cf 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindKeylessEntitiesQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindKeylessEntitiesQueryCosmosTest.cs @@ -171,6 +171,30 @@ FROM root c """); } + public override async Task Count_over_keyless_entity(bool async) + { + await base.Count_over_keyless_entity(async); + + AssertSql( +""" +SELECT COUNT(1) AS c +FROM root c +WHERE (c["Discriminator"] = "Customer") +"""); + } + + public override async Task Count_over_keyless_entity_with_pushdown(bool async) + { + // Cosmos client evaluation. Issue #17246. + await AssertTranslationFailed(() => base.Count_over_keyless_entity_with_pushdown(async)); + } + + public override async Task Count_over_keyless_entity_with_pushdown_empty_projection(bool async) + { + // Cosmos client evaluation. Issue #17246. + await AssertTranslationFailed(() => base.Count_over_keyless_entity_with_pushdown_empty_projection(async)); + } + private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected); diff --git a/test/EFCore.Specification.Tests/Query/NorthwindKeylessEntitiesQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindKeylessEntitiesQueryTestBase.cs index 30aecb7fa77..888b9fed440 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindKeylessEntitiesQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindKeylessEntitiesQueryTestBase.cs @@ -167,4 +167,25 @@ public virtual Task Collection_correlated_with_keyless_entity_in_predicate_works .Select(pv => new { pv.City, pv.ContactName }) .OrderBy(x => x.ContactName) .Take(2)); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Count_over_keyless_entity(bool async) + => AssertCount( + async, + ss => ss.Set()); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Count_over_keyless_entity_with_pushdown(bool async) + => AssertCount( + async, + ss => ss.Set().OrderBy(x => x.ContactTitle).Take(10)); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Count_over_keyless_entity_with_pushdown_empty_projection(bool async) + => AssertCount( + async, + ss => ss.Set().Take(10)); } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindKeylessEntitiesQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindKeylessEntitiesQuerySqlServerTest.cs index b8d928af77d..16b49821dcf 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindKeylessEntitiesQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindKeylessEntitiesQuerySqlServerTest.cs @@ -232,6 +232,56 @@ public override async Task KeylessEntity_with_included_nav(bool async) """); } + public override async Task Count_over_keyless_entity(bool async) + { + await base.Count_over_keyless_entity(async); + + AssertSql( +""" +SELECT COUNT(*) +FROM ( + 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] +) AS [m] +"""); + } + + public override async Task Count_over_keyless_entity_with_pushdown(bool async) + { + await base.Count_over_keyless_entity_with_pushdown(async); + + AssertSql( +""" +@__p_0='10' + +SELECT COUNT(*) +FROM ( + SELECT TOP(@__p_0) [m].[ContactTitle] + FROM ( + 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] + ) AS [m] + ORDER BY [m].[ContactTitle] +) AS [t] +"""); + } + + public override async Task Count_over_keyless_entity_with_pushdown_empty_projection(bool async) + { + await base.Count_over_keyless_entity_with_pushdown_empty_projection(async); + + AssertSql( +""" +@__p_0='10' + +SELECT COUNT(*) +FROM ( + SELECT TOP(@__p_0) 1 AS empty + FROM ( + 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] + ) AS [m] +) AS [t] +"""); + } + private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);