efcore.pg icon indicating copy to clipboard operation
efcore.pg copied to clipboard

Query generator build incorrect sql

Open vahpetr opened this issue 3 years ago • 3 comments

Query generator build incorrect sql.

Incorrect example:

var groupings = _dbContext.Activities.AsNoTracking()
              .GroupBy(p => new {p.Actor, p.Type})
              .OrderBy(p => p.Key.Actor)
                .ThenBy(p => p.Key.Type)
              .Select(g => new[] {g.Key.Actor, g.Key.Type, g.Count().ToString()});

Incorrect example result:

SELECT ARRAY[COUNT(*)::INT::text,COUNT(*)::INT::text,COUNT(*)::INT::text]::text[]
      FROM productivity.activities AS a
      GROUP BY a.actor, a.type
      ORDER BY a.actor, a.type

Was expecting something like this:

SELECT ARRAY[a.actor, a.type, COUNT(*)::text]::character varying(128)[]
      FROM productivity.activities AS a
      GROUP BY a.actor, a.type
      ORDER BY a.actor, a.type

Parts of code:

public class Activity : IDisposable
{
    public LocalDateTime Time { get; set; }
    public string Actor { get; set; } = null!;
    public string Type { get; set; } = null!;
    public JsonDocument? Meta { get; set; }

    public void Dispose() => Meta?.Dispose();
}

protected override void OnModelCreating(ModelBuilder builder)
    {
        base.OnModelCreating(builder);

        builder.HasPostgresExtension("pg_trgm");

        builder.Entity<Activity>(b =>
        {
            b.HasKey(p => new {p.Time, p.Actor, p.Type});
            b.HasAlternateKey(p => new {p.Time, p.Type, p.Actor});
            b.Property(p => p.Actor).HasMaxLength(128);
            b.Property(p => p.Type).HasMaxLength(32);
            b.HasIndex(p => p.Type).HasMethod("gin").HasOperators("gin_trgm_ops");
            b.HasIndex(p => p.Meta).HasMethod("gin").HasOperators("jsonb_path_ops");
        });

        builder.HasDefaultSchema("Productivity");

        builder.ToSnakeCase();
    }

For example, next code generate correct query:

var groupings = query
            .GroupBy(p => new {p.Actor, p.Type})
            .OrderBy(p => p.Key.Actor)
                .ThenBy(p => p.Key.Type)
            .Select(g => new[] {g.Key.Actor, g.Key.Type});

Correct output query result:

SELECT ARRAY[a.actor,a.type]::character varying(128)[]
      FROM productivity.activities AS a
      GROUP BY a.actor, a.type
      ORDER BY a.actor, a.type

And another example:

var groupings = query
            .GroupBy(p => new {p.Actor, p.Type})
            .OrderBy(p => p.Key.Actor)
                .ThenBy(p => p.Key.Type)
            .Select(g => new Grouping(new[] {g.Key.Actor, g.Key.Type}, g.Count()));

public record Grouping(string[] Keys, int Count)
{
    public override string ToString()
    {
        return $"{{ Keys = {Keys}, Count = {Count} }}";
    }
}

Correct output query result:

SELECT ARRAY[a.actor,a.type]::character varying(128)[], COUNT(*)::INT
      FROM productivity.activities AS a
      GROUP BY a.actor, a.type
      ORDER BY a.actor, a.type

Details:

TargetFramework 6.0

dotnet --info
.NET SDK (reflecting any global.json):
 Version:   6.0.302
 Commit:    c857713418

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  11.6
 OS Platform: Darwin
 RID:         osx.11.0-x64
 Base Path:   /usr/local/share/dotnet/sdk/6.0.302/

Host:
  Version:      6.0.7
  Architecture: x64
  Commit:       0ec02c8c96

.NET SDKs installed:
  3.1.413 [/usr/local/share/dotnet/sdk]
  3.1.414 [/usr/local/share/dotnet/sdk]
  3.1.415 [/usr/local/share/dotnet/sdk]
  5.0.401 [/usr/local/share/dotnet/sdk]
  5.0.402 [/usr/local/share/dotnet/sdk]
  5.0.403 [/usr/local/share/dotnet/sdk]
  6.0.100-rc.1.21458.32 [/usr/local/share/dotnet/sdk]
  6.0.100 [/usr/local/share/dotnet/sdk]
  6.0.101 [/usr/local/share/dotnet/sdk]
  6.0.201 [/usr/local/share/dotnet/sdk]
  6.0.302 [/usr/local/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 3.1.19 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.20 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.21 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.10 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.11 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.12 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.0-rc.1.21452.15 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.0 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.1 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.3 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.7 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 3.1.19 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.20 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.21 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.10 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.11 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.12 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.0-rc.1.21451.13 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.0 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.1 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.3 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.7 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]

Dependencies:
    <PackageReference Include="Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore" Version="6.0.7" />
    <PackageReference Include="Microsoft.Build.Framework" Version="17.2.0" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.InMemory" Version="6.0.7" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="6.0.7" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.Tools" PrivateAssets="All" Version="6.0.7" />
    <PackageReference Include="NodaTime" Version="3.1.0" />
    <PackageReference Include="NodaTime.Serialization.SystemTextJson" Version="1.0.0" />
    <PackageReference Include="Npgsql" Version="6.0.6" />
    <PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="6.0.5" />
    <PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL.Design" Version="1.1.1" />
    <PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime" Version="6.0.5" />
    <PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL.Trigrams" Version="5.0.10" />

vahpetr avatar Aug 07 '22 19:08 vahpetr

If add call ToString on first element of array:

var groupings = query
  .GroupBy(p => new {p.Actor, p.Type})
  .OrderBy(p => p.Key.Actor)
    .ThenBy(p => p.Key.Type)
  .Select(g => new[] {g.Key.Actor.ToString(), g.Key.Type, g.Count().ToString()});

I get correct sql:

SELECT a.actor, a.type, COUNT(*)::INT::text
      FROM productivity.activities AS a
      GROUP BY a.actor, a.type
      ORDER BY a.actor, a.type

vahpetr avatar Aug 07 '22 21:08 vahpetr

I'm unable to reproduce this, please see the minimal code sample below based on your fragments, which generates the following SQL:

SELECT ARRAY[a."Actor",a."Type",count(*)::int::text]::text[]
FROM "Activities" AS a
GROUP BY a."Actor", a."Type"
ORDER BY a."Actor", a."Type"

Can you please tweak the code sample to show the incorrect behavior?

Attempted repro
await using var ctx = new BlogContext();
await ctx.Database.EnsureDeletedAsync();
await ctx.Database.EnsureCreatedAsync();

var groupings = ctx.Activities.AsNoTracking()
    .GroupBy(p => new {p.Actor, p.Type})
    .OrderBy(p => p.Key.Actor)
    .ThenBy(p => p.Key.Type)
    .Select(g => new[] {g.Key.Actor, g.Key.Type, g.Count().ToString()})
    .ToList();

public class BlogContext : DbContext
{
    public DbSet<Activity> Activities { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseNpgsql(@"Host=localhost;Username=test;Password=test",
                o => o.UseNodaTime())
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Activity>(b =>
        {
            b.HasKey(p => new {p.Time, p.Actor, p.Type});
            b.HasAlternateKey(p => new {p.Time, p.Type, p.Actor});
            b.Property(p => p.Actor).HasMaxLength(128);
            b.Property(p => p.Type).HasMaxLength(32);
            // b.HasIndex(p => p.Type).HasMethod("gin").HasOperators("gin_trgm_ops");
            // b.HasIndex(p => p.Meta).HasMethod("gin").HasOperators("jsonb_path_ops");
        });
    }
}

public class Activity : IDisposable
{
    public LocalDateTime Time { get; set; }
    public string Actor { get; set; } = null!;
    public string Type { get; set; } = null!;
    public JsonDocument? Meta { get; set; }

    public void Dispose() => Meta?.Dispose();
}

roji avatar Aug 08 '22 10:08 roji

Ah, that was running with 7.0.0-preview.6. With 6.0.7 I indeed get the following instead:

SELECT ARRAY[COUNT(*)::INT::text,COUNT(*)::INT::text,COUNT(*)::INT::text]::text[]
...

So this seems like an EF-side GroupBy bug fixed for 7.0 (/cc @smitpatel)

roji avatar Aug 08 '22 10:08 roji

Closing as this is fixed in EF 7.0, and it isn't something we can backport to 6.0.

roji avatar Mar 09 '23 11:03 roji