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 24, 2023
1 parent bdaa665 commit 6fd586c
Show file tree
Hide file tree
Showing 4 changed files with 236 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/EFCore.Relational/Query/QuerySqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ protected override Expression VisitSelect(SelectExpression selectExpression)
else
{
_relationalCommandBuilder.Append("1");
if (selectExpression.Alias != null)
{
_relationalCommandBuilder.Append(" AS empty");
}
}

if (selectExpression.Tables.Any())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.EntityFrameworkCore.Query;

public abstract class KeylessEntitiesQueryAdHocTestBase : NonSharedModelTestBase
{
protected KeylessEntitiesQueryAdHocTestBase(ITestOutputHelper testOutputHelper)
{
//TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
}

protected override string StoreName
=> "KeylessEntitiesQueryAdHocTest";

#region 29667

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Count_over_keyless_entities(bool async)
{
var contextFactory = await Initialize29667();
using var context = contextFactory.CreateContext();
var result = async
? await context.KeylessEntities.CountAsync()
: context.KeylessEntities.Count();

Assert.Equal(5, result);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Count_over_keyless_entities_with_pushdown(bool async)
{
var contextFactory = await Initialize29667();
using var context = contextFactory.CreateContext();

var query = context.KeylessEntities.OrderBy(x => x.Number).Take(10);
var result = async
? await query.CountAsync()
: query.Count();

Assert.Equal(5, result);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Count_over_keyless_entities_with_pushdown_empty_projection(bool async)
{
var contextFactory = await Initialize29667();
using var context = contextFactory.CreateContext();

var query = context.KeylessEntities.Take(10);
var result = async
? await query.CountAsync()
: query.Count();

Assert.Equal(5, result);
}

protected abstract void Seed29667(MyContext29667 ctx);

protected Task<ContextFactory<MyContext29667>> Initialize29667()
=> InitializeAsync<MyContext29667>(
seed: Seed29667,
onConfiguring: ob => ob.ConfigureWarnings(x => x.Ignore(CoreEventId.RowLimitingOperationWithoutOrderByWarning)));

protected class MyContext29667 : DbContext
{
public MyContext29667(DbContextOptions options)
: base(options)
{
}

public DbSet<MyKeylessEntity29667> KeylessEntities { get; set; }

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<MyKeylessEntity29667>().HasNoKey();
}
}

public class MyKeylessEntity29667
{
public string Name { get; set; }
public int? Number { get; set; }
}

#endregion
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.EntityFrameworkCore.Query;

public class KeylessEntitiesQueryAdHocSqlServerTest : KeylessEntitiesQueryAdHocTestBase
{
public KeylessEntitiesQueryAdHocSqlServerTest(ITestOutputHelper testOutputHelper)
: base(testOutputHelper)
{
}

protected override ITestStoreFactory TestStoreFactory
=> SqlServerTestStoreFactory.Instance;

public override async Task Count_over_keyless_entities(bool async)
{
await base.Count_over_keyless_entities(async);

AssertSql(
"""
SELECT COUNT(*)
FROM [KeylessEntities] AS [k]
""");
}

public override async Task Count_over_keyless_entities_with_pushdown(bool async)
{
await base.Count_over_keyless_entities_with_pushdown(async);

AssertSql(
"""
@__p_0='10'
SELECT COUNT(*)
FROM (
SELECT TOP(@__p_0) [k].[Number]
FROM [KeylessEntities] AS [k]
ORDER BY [k].[Number]
) AS [t]
""");
}

public override async Task Count_over_keyless_entities_with_pushdown_empty_projection(bool async)
{
await base.Count_over_keyless_entities_with_pushdown_empty_projection(async);

AssertSql(
"""
@__p_0='10'
SELECT COUNT(*)
FROM (
SELECT TOP(@__p_0) 1 AS empty
FROM [KeylessEntities] AS [k]
) AS [t]
""");
}

protected override void Seed29667(MyContext29667 ctx)
{
ctx.Database.ExecuteSqlRaw(@"INSERT INTO [KeylessEntities] ([Name], [Number]) VALUES
(N'k1', 1), (N'k1', 2), (N'k1', 3), (N'k2', 1), (N'k2', 1)");
}

private void AssertSql(params string[] expected)
=> ((TestSqlLoggerFactory)ServiceProvider.GetRequiredService<ILoggerFactory>()).AssertBaseline(expected);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.EntityFrameworkCore.Query;

public class KeylessEntitiesQueryAdHocSqliteTest : KeylessEntitiesQueryAdHocTestBase
{
public KeylessEntitiesQueryAdHocSqliteTest(ITestOutputHelper testOutputHelper)
: base(testOutputHelper)
{
}

protected override ITestStoreFactory TestStoreFactory
=> SqliteTestStoreFactory.Instance;

public override async Task Count_over_keyless_entities(bool async)
{
await base.Count_over_keyless_entities(async);

AssertSql(
"""
SELECT COUNT(*)
FROM "KeylessEntities" AS "k"
""");
}

public override async Task Count_over_keyless_entities_with_pushdown(bool async)
{
await base.Count_over_keyless_entities_with_pushdown(async);

AssertSql(
"""
@__p_0='10'
SELECT COUNT(*)
FROM (
SELECT "k"."Number"
FROM "KeylessEntities" AS "k"
ORDER BY "k"."Number"
LIMIT @__p_0
) AS "t"
""");
}

public override async Task Count_over_keyless_entities_with_pushdown_empty_projection(bool async)
{
await base.Count_over_keyless_entities_with_pushdown_empty_projection(async);

AssertSql(
"""
@__p_0='10'
SELECT COUNT(*)
FROM (
SELECT 1 AS empty
FROM "KeylessEntities" AS "k"
LIMIT @__p_0
) AS "t"
""");
}

protected override void Seed29667(MyContext29667 ctx)
{
ctx.Database.ExecuteSqlRaw(@"INSERT INTO [KeylessEntities] ([Name], [Number]) VALUES
('k1', 1), ('k1', 2), ('k1', 3), ('k2', 1), ('k2', 1)");
//ctx.Database.ExecuteSqlRaw(@"INSERT INTO [KeylessEntities] ([Name], [Number]) VALUES(N'k1', 2)");
//ctx.Database.ExecuteSqlRaw(@"INSERT INTO [KeylessEntities] ([Name], [Number]) VALUES(N'k1', 3)");
//ctx.Database.ExecuteSqlRaw(@"INSERT INTO [KeylessEntities] ([Name], [Number]) VALUES(N'k2', 1)");
//ctx.Database.ExecuteSqlRaw(@"INSERT INTO [KeylessEntities] ([Name], [Number]) VALUES(N'k2', 2)");
}

private void AssertSql(params string[] expected)
=> ((TestSqlLoggerFactory)ServiceProvider.GetRequiredService<ILoggerFactory>()).AssertBaseline(expected);
}

0 comments on commit 6fd586c

Please sign in to comment.