Skip to content

Commit ac4ce67

Browse files
kevin-dpclaudeautofix-ci[bot]
authored
fix(db): support aggregates nested inside expressions (#720) (#1274)
* test(db, react-db): reproduce aggregates nested inside expressions Add failing tests that reproduce the issue where aggregates wrapped inside other expressions (e.g. coalesce(count(...), 0)) throw QueryCompilationError: Unknown expression type: agg. Tests cover coalesce wrapping count/sum, add combining two aggregates, mixed plain and wrapped aggregates, and subquery join sources — in both @tanstack/db and useLiveQuery. Ref: #720 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ci: apply automated fixes * fix(db): support aggregates nested inside expressions Extract nested aggregates (e.g. coalesce(count(...), 0)) into synthetic aliases, register them with the groupBy operator, and evaluate the wrapper expression post-aggregation. This follows the same pattern used for HAVING clauses via replaceAggregatesByRefs. Changes: - select.ts: defer expressions containing nested aggregates to groupBy - group-by.ts: extract, register, and evaluate wrapped aggregates in both single-group and multi-group paths - index.ts: detect nested aggregates for implicit single-group aggregation Fixes: #720 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ci: apply automated fixes * chore: add changeset Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ci: apply automated fixes * fix(db): clean up synthetic __agg_ keys from result rows Remove temporary __agg_N keys from finalResults after wrapped-aggregate expressions have been evaluated. Without this cleanup, internal synthetic aliases leak onto user-visible projected result rows. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(db): extract evaluateWrappedAggregates helper Deduplicate the synthetic-value copy, evaluate, and cleanup logic that was repeated in both the single-group and multi-group paths of processGroupBy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(db, react-db): assert exact result shape for nested aggregates Use toEqual to verify that projected rows contain only the expected keys and no leaked internal state like __agg_N synthetic aliases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent 306ddd3 commit ac4ce67

6 files changed

Lines changed: 430 additions & 18 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/db': patch
3+
---
4+
5+
fix: support aggregates nested inside expressions (e.g. `coalesce(count(...), 0)`)

packages/db/src/query/compiler/group-by.ts

Lines changed: 131 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
map,
66
serializeValue,
77
} from '@tanstack/db-ivm'
8-
import { Func, PropRef, getHavingExpression } from '../ir.js'
8+
import { Func, PropRef, getHavingExpression, isExpressionLike } from '../ir.js'
99
import {
1010
AggregateFunctionNotInSelectError,
1111
NonAggregateExpressionNotInGroupByError,
@@ -49,8 +49,8 @@ function validateAndCreateMapping(
4949

5050
// Validate each SELECT expression
5151
for (const [alias, expr] of Object.entries(selectClause)) {
52-
if (expr.type === `agg`) {
53-
// Aggregate expressions are allowed and don't need to be in GROUP BY
52+
if (expr.type === `agg` || containsAggregate(expr)) {
53+
// Aggregate expressions (plain or wrapped) are allowed and don't need to be in GROUP BY
5454
continue
5555
}
5656

@@ -86,12 +86,26 @@ export function processGroupBy(
8686
// For single-group aggregation, create a single group with all data
8787
const aggregates: Record<string, any> = {}
8888

89+
// Expressions that wrap aggregates (e.g. coalesce(count(...), 0)).
90+
// Keys are the original SELECT aliases; values are pre-compiled evaluators
91+
// over the transformed (aggregate-free) expression.
92+
const wrappedAggExprs: Record<string, (data: any) => any> = {}
93+
const aggCounter = { value: 0 }
94+
8995
if (selectClause) {
9096
// Scan the SELECT clause for aggregate functions
9197
for (const [alias, expr] of Object.entries(selectClause)) {
9298
if (expr.type === `agg`) {
93-
const aggExpr = expr
94-
aggregates[alias] = getAggregateFunction(aggExpr)
99+
aggregates[alias] = getAggregateFunction(expr)
100+
} else if (containsAggregate(expr)) {
101+
const { transformed, extracted } = extractAndReplaceAggregates(
102+
expr as BasicExpression | Aggregate,
103+
aggCounter,
104+
)
105+
for (const [syntheticAlias, aggExpr] of Object.entries(extracted)) {
106+
aggregates[syntheticAlias] = getAggregateFunction(aggExpr)
107+
}
108+
wrappedAggExprs[alias] = compileExpression(transformed)
95109
}
96110
}
97111
}
@@ -112,13 +126,17 @@ export function processGroupBy(
112126
const finalResults: Record<string, any> = { ...selectResults }
113127

114128
if (selectClause) {
115-
// Update with aggregate results
129+
// First pass: populate plain aggregate results and synthetic aliases
116130
for (const [alias, expr] of Object.entries(selectClause)) {
117131
if (expr.type === `agg`) {
118132
finalResults[alias] = aggregatedRow[alias]
119133
}
120-
// Non-aggregates keep their original values from early SELECT processing
121134
}
135+
evaluateWrappedAggregates(
136+
finalResults,
137+
aggregatedRow as Record<string, any>,
138+
wrappedAggExprs,
139+
)
122140
}
123141

124142
// Use a single key for the result and update $selected
@@ -201,13 +219,23 @@ export function processGroupBy(
201219

202220
// Create aggregate functions for any aggregated columns in the SELECT clause
203221
const aggregates: Record<string, any> = {}
222+
const wrappedAggExprs: Record<string, (data: any) => any> = {}
223+
const aggCounter = { value: 0 }
204224

205225
if (selectClause) {
206226
// Scan the SELECT clause for aggregate functions
207227
for (const [alias, expr] of Object.entries(selectClause)) {
208228
if (expr.type === `agg`) {
209-
const aggExpr = expr
210-
aggregates[alias] = getAggregateFunction(aggExpr)
229+
aggregates[alias] = getAggregateFunction(expr)
230+
} else if (containsAggregate(expr)) {
231+
const { transformed, extracted } = extractAndReplaceAggregates(
232+
expr as BasicExpression | Aggregate,
233+
aggCounter,
234+
)
235+
for (const [syntheticAlias, aggExpr] of Object.entries(extracted)) {
236+
aggregates[syntheticAlias] = getAggregateFunction(aggExpr)
237+
}
238+
wrappedAggExprs[alias] = compileExpression(transformed)
211239
}
212240
}
213241
}
@@ -223,9 +251,11 @@ export function processGroupBy(
223251
const finalResults: Record<string, any> = {}
224252

225253
if (selectClause) {
226-
// Process each SELECT expression
254+
// First pass: populate group keys, plain aggregates, and synthetic aliases
227255
for (const [alias, expr] of Object.entries(selectClause)) {
228-
if (expr.type !== `agg`) {
256+
if (expr.type === `agg`) {
257+
finalResults[alias] = aggregatedRow[alias]
258+
} else if (!wrappedAggExprs[alias]) {
229259
// Use cached mapping to get the corresponding __key_X for non-aggregates
230260
const groupIndex = mapping.selectToGroupByIndex.get(alias)
231261
if (groupIndex !== undefined) {
@@ -234,11 +264,13 @@ export function processGroupBy(
234264
// Fallback to original SELECT results
235265
finalResults[alias] = selectResults[alias]
236266
}
237-
} else {
238-
// Use aggregate results
239-
finalResults[alias] = aggregatedRow[alias]
240267
}
241268
}
269+
evaluateWrappedAggregates(
270+
finalResults,
271+
aggregatedRow as Record<string, any>,
272+
wrappedAggExprs,
273+
)
242274
} else {
243275
// No SELECT clause - just use the group keys
244276
for (let i = 0; i < groupByClause.length; i++) {
@@ -457,6 +489,91 @@ export function replaceAggregatesByRefs(
457489
}
458490
}
459491

492+
/**
493+
* Evaluates wrapped-aggregate expressions against the aggregated row.
494+
* Copies synthetic __agg_N values into finalResults so the compiled wrapper
495+
* expressions can reference them, evaluates each wrapper, then removes the
496+
* synthetic keys so they don't leak onto user-visible result rows.
497+
*/
498+
function evaluateWrappedAggregates(
499+
finalResults: Record<string, any>,
500+
aggregatedRow: Record<string, any>,
501+
wrappedAggExprs: Record<string, (data: any) => any>,
502+
): void {
503+
for (const key of Object.keys(aggregatedRow)) {
504+
if (key.startsWith(`__agg_`)) {
505+
finalResults[key] = aggregatedRow[key]
506+
}
507+
}
508+
for (const [alias, evaluator] of Object.entries(wrappedAggExprs)) {
509+
finalResults[alias] = evaluator({ $selected: finalResults })
510+
}
511+
for (const key of Object.keys(finalResults)) {
512+
if (key.startsWith(`__agg_`)) delete finalResults[key]
513+
}
514+
}
515+
516+
/**
517+
* Checks whether an expression contains an aggregate anywhere in its tree.
518+
* Returns true for a top-level Aggregate, or a Func whose args (recursively)
519+
* contain an Aggregate. Safely returns false for nested Select objects.
520+
*/
521+
export function containsAggregate(
522+
expr: BasicExpression | Aggregate | Select,
523+
): boolean {
524+
if (!isExpressionLike(expr)) {
525+
return false
526+
}
527+
if (expr.type === `agg`) {
528+
return true
529+
}
530+
if (expr.type === `func`) {
531+
return expr.args.some((arg: BasicExpression | Aggregate) =>
532+
containsAggregate(arg),
533+
)
534+
}
535+
return false
536+
}
537+
538+
/**
539+
* Walks an expression tree containing nested aggregates.
540+
* Each Aggregate node is extracted, assigned a synthetic alias (__agg_N),
541+
* and replaced with PropRef(["$selected", "__agg_N"]) so the wrapper
542+
* expression can be compiled as a pure BasicExpression after groupBy
543+
* populates the synthetic values.
544+
*/
545+
function extractAndReplaceAggregates(
546+
expr: BasicExpression | Aggregate,
547+
counter: { value: number },
548+
): {
549+
transformed: BasicExpression
550+
extracted: Record<string, Aggregate>
551+
} {
552+
if (expr.type === `agg`) {
553+
const alias = `__agg_${counter.value++}`
554+
return {
555+
transformed: new PropRef([`$selected`, alias]),
556+
extracted: { [alias]: expr },
557+
}
558+
}
559+
560+
if (expr.type === `func`) {
561+
const allExtracted: Record<string, Aggregate> = {}
562+
const newArgs = expr.args.map((arg: BasicExpression | Aggregate) => {
563+
const result = extractAndReplaceAggregates(arg, counter)
564+
Object.assign(allExtracted, result.extracted)
565+
return result.transformed
566+
})
567+
return {
568+
transformed: new Func(expr.name, newArgs),
569+
extracted: allExtracted,
570+
}
571+
}
572+
573+
// ref / val – pass through unchanged
574+
return { transformed: expr as BasicExpression, extracted: {} }
575+
}
576+
460577
/**
461578
* Checks if two aggregate expressions are equal
462579
*/

packages/db/src/query/compiler/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
import { PropRef, Value as ValClass, getWhereExpression } from '../ir.js'
1212
import { compileExpression, toBooleanPredicate } from './evaluators.js'
1313
import { processJoins } from './joins.js'
14-
import { processGroupBy } from './group-by.js'
14+
import { containsAggregate, processGroupBy } from './group-by.js'
1515
import { processOrderBy } from './order-by.js'
1616
import { processSelect } from './select.js'
1717
import type { CollectionSubscription } from '../../collection/subscription.js'
@@ -268,7 +268,7 @@ export function compileQuery(
268268
} else if (query.select) {
269269
// Check if SELECT contains aggregates but no GROUP BY (implicit single-group aggregation)
270270
const hasAggregates = Object.values(query.select).some(
271-
(expr) => expr.type === `agg`,
271+
(expr) => expr.type === `agg` || containsAggregate(expr),
272272
)
273273
if (hasAggregates) {
274274
// Handle implicit single-group aggregation

packages/db/src/query/compiler/select.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { map } from '@tanstack/db-ivm'
22
import { PropRef, Value as ValClass, isExpressionLike } from '../ir.js'
33
import { AggregateNotSupportedError } from '../../errors.js'
44
import { compileExpression } from './evaluators.js'
5+
import { containsAggregate } from './group-by.js'
56
import type { Aggregate, BasicExpression, Select } from '../ir.js'
67
import type {
78
KeyedStream,
@@ -226,8 +227,10 @@ function addFromObject(
226227
continue
227228
}
228229

229-
if (isAggregateExpression(expression)) {
230-
// Placeholder for group-by processing later
230+
if (isAggregateExpression(expression) || containsAggregate(expression)) {
231+
// Placeholder for group-by processing later.
232+
// Both plain aggregates (count(...)) and expressions wrapping
233+
// aggregates (coalesce(count(...), 0)) are deferred to processGroupBy.
231234
ops.push({
232235
kind: `field`,
233236
alias: [...prefixPath, key].join(`.`),

0 commit comments

Comments
 (0)