Skip to content

Commit

Permalink
Fix to #29667 - Count after Take throws "No column name was specified…
Browse files Browse the repository at this point in the history
… for column 1 of 't'."

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
  • Loading branch information
maumar committed Jan 25, 2023
1 parent f8a5d54 commit dfe2bf6
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 1 deletion.
11 changes: 10 additions & 1 deletion src/EFCore.Relational/Query/QuerySqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ protected override Expression VisitSelect(SelectExpression selectExpression)
}
else
{
_relationalCommandBuilder.Append("1");
GenerateEmptyProjection(selectExpression);
}

if (selectExpression.Tables.Any())
Expand Down Expand Up @@ -306,6 +306,15 @@ protected virtual void GeneratePseudoFromClause()
{
}

/// <summary>
/// Generates empty projection for a SelectExpression.
/// </summary>
/// <param name="selectExpression">SelectExpression for which the empty projection will be generated.</param>
protected virtual void GenerateEmptyProjection(SelectExpression selectExpression)
{
_relationalCommandBuilder.Append("1");
}

/// <inheritdoc />
protected override Expression VisitProjection(ProjectionExpression projectionExpression)
{
Expand Down
15 changes: 15 additions & 0 deletions src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,21 @@ protected override Expression VisitUpdate(UpdateExpression updateExpression)
RelationalStrings.ExecuteOperationWithUnsupportedOperatorInSqlGeneration(nameof(RelationalQueryableExtensions.ExecuteUpdate)));
}

/// <summary>
/// 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.
/// </summary>
protected override void GenerateEmptyProjection(SelectExpression selectExpression)
{
base.GenerateEmptyProjection(selectExpression);
if (selectExpression.Alias != null)
{
Sql.Append(" AS empty");
}
}

/// <summary>
/// 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<CustomerQuery>());

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Count_over_keyless_entity_with_pushdown(bool async)
=> AssertCount(
async,
ss => ss.Set<CustomerQuery>().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<CustomerQuery>().Take(10));
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit dfe2bf6

Please sign in to comment.