Skip to content

Render aggregates over expressions (e.g. sumBy(caseWhen …)) instead of throwing#132

Merged
JordanMarr merged 1 commit into
JordanMarr:beta-4.1from
michaelglass:fix/aggregate-over-expression
Jun 10, 2026
Merged

Render aggregates over expressions (e.g. sumBy(caseWhen …)) instead of throwing#132
JordanMarr merged 1 commit into
JordanMarr:beta-4.1from
michaelglass:fix/aggregate-over-expression

Conversation

@michaelglass

@michaelglass michaelglass commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

whoops, one more bug from me...

sumBy/avgBy/etc. over anything other than a bare column threw Invalid argument to aggregate function — even though NAggregateColumn's own doc comment says such expressions should fall through to full expression rendering. So a conditional count like SUM(CASE WHEN … THEN 1 ELSE 0 END) was impossible.

AggregateColumn/NAggregateColumn now return None for a non-column argument, letting it render as an expression. The Option/Nullable .Value chain is unaffected — it's still matched by the Property arm.

Tests: sumBy/avgBy over caseWhen now render SUM(CASE WHEN …) / AVG(CASE WHEN …).

…r throws

AggregateColumn/NAggregateColumn threw "Invalid argument to aggregate
function" whenever an aggregate's argument was not a bare column. This
contradicted NAggregateColumn's own doc comment, which states that
aggregates over arbitrary expressions should fall through to expression
rendering. The throw broke valid conditional-aggregate queries such as
SUM(CASE WHEN ... THEN 1 ELSE 0 END).

Return None instead so the expression falls through to full rendering.
The Option/Nullable .Value chain is already handled by the Property arm.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@JordanMarr

Copy link
Copy Markdown
Owner
    let sql =
        select {
            for p in production.product do
            groupBy p.color
            select (sumBy (caseWhen (p.standardcost > 100m) 1 0))
        }
        |> toSql

That select statement is doing some work lol

@JordanMarr JordanMarr merged commit be16d1b into JordanMarr:beta-4.1 Jun 10, 2026
1 check passed
michaelglass added a commit to michaelglass/SqlHydra.Query.Pgvector that referenced this pull request Jun 11, 2026
…n fix, JordanMarr/SqlHydra#132)

Also bump test/dev dependencies to latest: Npgsql 9.0.4 -> 10.0.3,
Microsoft.Testing.Extensions.CodeCoverage 18.5.2 -> 18.8.0, dotnet-fsharplint
0.26.10 -> 0.27.0, fsdocs-tool 21.0.0 -> 22.1.0, coverageratchet alpha.6 -> alpha.7.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants