Stop referring to ungrouped columns from outer query in subquery

[roji]

PostgreSQL seems to be particularly strict around subqueries and GROUP BY. The following query (full repro below):

from s in (from i in ctx.Invoices
        group i by i.History.Month
        into g
        select new
        {
            Month = g.Key,
            Total = g.Sum(p => p.Amount),
        })
    select new
    {
        s.Month,
        s.Total,
        Payment = ctx.Payments.Where(p => p.History.Month == s.Month).Sum(p => p.Amount)
    }

... generates the following SQL on PostgreSQL:

SELECT date_part('month', i."History")::INT AS "Month", COALESCE(SUM(i."Amount"), 0)::INT AS "Total", (
    SELECT COALESCE(SUM(p."Amount"), 0)::INT
    FROM "Payments" AS p
    WHERE date_part('month', p."History")::INT = date_part('month', i."History")::INT) AS "Payment"
FROM "Invoices" AS i
GROUP BY date_part('month', i."History")::INT

... and errors with the following: subquery uses ungrouped column "i.History" from outer query.

The same works fine on SQL Server, SQLite and MariaDB.

Note also that the current SQL is less efficient even when supported, since the same expression gets evaluated more than once (see #27266 (comment)).

See suggested alternative SQL by @mojtabakaviani in npgsql/efcore.pg#2236 (comment).

Full repro

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

var query = from s in (from i in ctx.Invoices
        group i by i.History.Month
        into g
        select new
        {
            Month = g.Key,
            Total = g.Sum(p => p.Amount),
        })
    select new
    {
        s.Month,
        s.Total,
        Payment = ctx.Payments.Where(p => p.History.Month == s.Month).Sum(p => p.Amount)
    };

_ = query.ToList();

public class BlogContext : DbContext
{
    public DbSet<Invoice> Invoices { get; set; }
    public DbSet<Payment> Payments { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            // .UseSqlServer(@"Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0")
            .UseNpgsql(@"Host=localhost;Username=test;Password=test")
            // .UseSqlite("Data Source=/tmp/foo.sql")
            // .UseMySql("Server=localhost;user=test;password=test;database=test", new MariaDbServerVersion("10.5"))
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();
}

public class Invoice
{
    public int Id { get; set; }
    public int Amount { get; set; }

    public DateTime History { get; set; }
}

public class Payment
{
    public int Id { get; set; }
    public int Amount { get; set; }

    public DateTime History { get; set; }
}

Raised by @mojtabakaviani in npgsql/efcore.pg#2236

[smitpatel]

Cannot really do much, probably something which is just unsupported.

[mojtabakaviani]

Yes, but if EF Core avoid convert query to join work properly.

[smitpatel]

There is no conversion to join here.

[mojtabakaviani]

@smitpatel I mean, converting nested query to scalar result, for this example.

[smitpatel]

But there is no conversion EF Core performs here. It is nested query which happens to be scalar result. Every scalar results are same regardless of where they come from.

[roji]

Note @mojtabakaviani alternate SQL proposal in npgsql/efcore.pg#2236:

SELECT i0."Month", i0."Total", (
    SELECT COALESCE(SUM(p.amount), 0.0) FROM payments AS p
    WHERE date_part('month', p.history)::INT = i0."Month"::INT) AS "Payment"
FROM (
    SELECT date_part('month', i.history)::INT AS "Month", COALESCE(SUM(i.amount), 0.0) AS "Total"
    FROM invoices AS i
    GROUP BY date_part('month', i.history)::INT) AS i0;

Note that this is also better from a perf standpoint than what we currently produce (for any database), as the month is calculated only once rather than three times in the current query. If the GROUP BY key is a complex function or even a subquery, that could be very significant (see the similarity with #27285).

[mojtabakaviani]

Thanks @roji for proposal, I test and work in the both SQL Server and Postgresql. also solve nested query issue #27267

[roji]

@mojtabakaviani I ended up removing the WITH alternative since it doesn't really add anything that a simple subquery doesn't already accomplish (as in your original proposal).

[ajcvickers]

Note from triage: add tests for this case.

[roji]

@ajcvickers note that this has what could be an important perf aspect as well (multiple function/subquery evaluation which could be avoided), so this isn't just about finding a solution for PG. We could add a test here, and have another issue tracking changing the SQL we generate (which would both help perf and make the query work on PG).

[smitpatel]

Pending selector work covers that already.

[roji]

Great! In that case I guess this can just remind us to add the test.

[smitpatel]

Poaching this. Pending selector work is on hold now. #27433 has same issue for SqlServer in different scenario. Also #27132 work detected that we may not be able to lift always causing subquery. An ideal fix here would be to basically pushdown into subquery when grouping key is complex to avoid issues generated by subquery having correlation predicate with grouping key. Does that sound good to you @roji ?

[roji]

I think so - that would mean the key expression would be present only once in the SQL, instead of twice (as above), right? Sounds great.