feat(db): support custom aggregate functions - #1702
Open
jakeboone02 wants to merge 2 commits into
Open
Conversation
Add a global, case-insensitive registry for user-defined aggregates. The group-by compiler now looks up registrations before the built-in switch, so custom names work anywhere built-ins do — select, having, and orderBy via $selected — and built-ins can be overridden (warned about in dev) and restored by unregistering. Public API: - createAggregate(name, factory): registers and returns a typed builder, so the aggregate name is declared once and its result type flows into select() - registerAggregate / unregisterAggregate / getRegisteredAggregates for dynamic registration - toExpression, ExpressionLike and the Aggregate type are now exported for the low-level path Factories receive the raw value extractor plus the row key, letting aggregates such as group_concat stay deterministic despite preMap outputs being consolidated by value hash. Arguments after the first are evaluated once at compile time and must be constant, otherwise NonConstantAggregateArgumentError is thrown; UnsupportedAggregateFunctionError now lists registered names. Closes TanStack#1558
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCustom aggregate functions are now registerable through typed and low-level APIs, compiled ahead of built-in aggregates, usable across grouped query clauses, and covered by runtime and type tests. Documentation and minor-release metadata describe the new functionality. ChangesCustom aggregate support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant QueryBuilder
participant GroupByCompiler
participant CustomAggregateFactory
QueryBuilder->>GroupByCompiler: compile aggregate expression
GroupByCompiler->>GroupByCompiler: compile constant parameters
GroupByCompiler->>CustomAggregateFactory: invoke with value and key accessors
CustomAggregateFactory-->>GroupByCompiler: return aggregate implementation
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/db/tests/query/custom-aggregates.test.ts (2)
528-565: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate the repeated query-building block.
The same faulty query is built and run twice — once for
toThrow(UnsupportedAggregateFunctionError), once inside atry/catchto check the message. Both assertions can be made from a single execution. As per coding guidelines,**/*.{ts,tsx,js}should "extract common logic into utility functions when identical or near-identical code blocks appear in multiple places."♻️ Suggested consolidation
- const todos = createTodosCollection() - expect(() => - createLiveQueryCollection({ - startSync: true, - query: (q) => - q - .from({ todo: todos }) - .groupBy(({ todo }) => todo.listId) - .select(({ todo }) => ({ - listId: todo.listId, - value: new Aggregate(`nope`, [ - toExpression(todo.points), - ]) as any, - })), - }), - ).toThrow(UnsupportedAggregateFunctionError) - - try { - createLiveQueryCollection({ - startSync: true, - query: (q) => - q - .from({ todo: todos }) - .groupBy(({ todo }) => todo.listId) - .select(({ todo }) => ({ - listId: todo.listId, - value: new Aggregate(`nope`, [ - toExpression(todo.points), - ]) as any, - })), - }) - } catch (error) { - expect((error as Error).message).toContain(`known_agg`) - } + const todos = createTodosCollection() + const buildBadQuery = () => + createLiveQueryCollection({ + startSync: true, + query: (q) => + q + .from({ todo: todos }) + .groupBy(({ todo }) => todo.listId) + .select(({ todo }) => ({ + listId: todo.listId, + value: new Aggregate(`nope`, [ + toExpression(todo.points), + ]) as any, + })), + }) + + let caught: unknown + try { + buildBadQuery() + } catch (error) { + caught = error + } + expect(caught).toBeInstanceOf(UnsupportedAggregateFunctionError) + expect((caught as Error).message).toContain(`known_agg`)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/tests/query/custom-aggregates.test.ts` around lines 528 - 565, Deduplicate the repeated query construction in the test `unknown aggregate throws and lists registered names` by executing the faulty `createLiveQueryCollection` call once and capturing its thrown error. Assert that the captured error is an `UnsupportedAggregateFunctionError` and that its message contains `known_agg`, while preserving the existing query behavior.Source: Coding guidelines
73-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid
anyin theregistertest helper.
args: Array<any>plus(registerAggregate as any)(...)bypasses type checking on a helper used across ~15 test cases; a mistaken call signature wouldn't be caught. As per coding guidelines,**/*.{ts,tsx}should "useunknowninstead when the type is truly unknown, and provide proper type annotations for return values."♻️ Suggested typing
-function register(name: string, ...args: Array<any>) { +function register( + name: string, + ...args: Parameters<typeof registerAggregate> extends [string, ...infer Rest] + ? Rest + : never +) { registeredInTest.add(name.toLowerCase()) - return (registerAggregate as any)(name, ...args) + return registerAggregate(name, ...(args as Parameters<typeof registerAggregate>[1..])) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/tests/query/custom-aggregates.test.ts` around lines 73 - 76, Update the register test helper to remove both any usages: type variadic args as unknown (or with the actual aggregate registration parameter types), and give the helper an explicit return type matching registerAggregate without casting the function to any. Preserve the existing lowercase tracking and argument forwarding behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/db/tests/query/custom-aggregates.test.ts`:
- Around line 528-565: Deduplicate the repeated query construction in the test
`unknown aggregate throws and lists registered names` by executing the faulty
`createLiveQueryCollection` call once and capturing its thrown error. Assert
that the captured error is an `UnsupportedAggregateFunctionError` and that its
message contains `known_agg`, while preserving the existing query behavior.
- Around line 73-76: Update the register test helper to remove both any usages:
type variadic args as unknown (or with the actual aggregate registration
parameter types), and give the helper an explicit return type matching
registerAggregate without casting the function to any. Preserve the existing
lowercase tracking and argument forwarding behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 25dc9243-064f-4b52-a5ee-7702c810c717
📒 Files selected for processing (9)
.changeset/smart-pugs-listen.mddocs/guides/live-queries.mdpackages/db/src/errors.tspackages/db/src/query/aggregates.tspackages/db/src/query/builder/functions.tspackages/db/src/query/compiler/group-by.tspackages/db/src/query/index.tspackages/db/tests/query/custom-aggregates.test-d.tspackages/db/tests/query/custom-aggregates.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
Aggregate support in
packages/db/src/query/compiler/group-by.tswas a hardcoded switch oversum,count,avg,min,max; every other name threwUnsupportedAggregateFunctionError. Domain-specific aggregations (group_concat,array_agg, bitwise OR, geometric mean, …) required forking the package.This adds a registry for user-defined aggregates built on the existing
{ preMap, reduce, postMap? }contract from@tanstack/db-ivm.Public API
createAggregateregisters an aggregate and returns a typed builder forselect():A lower-level API is exported for dynamic/plugin scenarios:
Also newly exported to support the low-level path:
toExpression, theExpressionLiketype, and theAggregatetype (the class remains reachable asIR.Aggregate).Design notes
preMapoutputs are consolidated by hash insidedb-ivm'sIndex, so two rows producing the same value collapse into one entry with multiplicity 2, and iteration order is map order rather than row order. Passingctx.key(entry)lets aggregates such asgroup_concatkeep per-row identity and sort deterministically.ctx.valueis the raw value — no numeric coercion, unlike thesum/avgpath.reduceis a full recompute.ReduceOperatorpasses the complete consolidated multiset for the group on every change, so implementations need no incremental accumulator bookkeeping. Ignoringmultiplicityunder-counts duplicates; this is called out in the docs.getAggregateFunctionchecks the registry before the built-in switch. That is what allows overriding a built-in, and it also meansunregisterAggregate('sum')restores the built-in for free — no special-casing.paramstuple. A column reference there now throws the newNonConstantAggregateArgumentErrorinstead of silently evaluating toundefined.group_concat(SQL STRING_AGG function in groupBy #422) and the unusedmedian/modeoperators indb-ivmare intentionally out of scope; they are now expressible in user land.aggregatesEqualcompares name + args), nested-in-expression extraction (__agg_N), and ordering via$selected.<alias>.Files
packages/db/src/query/aggregates.tscreateAggregate, dev warningpackages/db/src/query/compiler/group-by.tspackages/db/src/errors.tsNonConstantAggregateArgumentError;UnsupportedAggregateFunctionErrornow lists registered namespackages/db/src/query/index.tspackages/db/src/query/builder/functions.tsExpressionLiketypepackages/db/tests/query/custom-aggregates.test.tspackages/db/tests/query/custom-aggregates.test-d.tsdocs/guides/live-queries.mdBackwards compatibility
Additive only.
UnsupportedAggregateFunctionError's constructor gains an optional second parameter; existing call sites and behavior are unchanged.✅ Checklist
pnpm test.Test coverage: registration/unregistration/case-insensitivity, snapshot semantics of
getRegisteredAggregates, re-registration and built-in override warnings,group_concatwith and without a custom separator, multiplicity of consolidated duplicates,postMap, raw (uncoerced) values, computed inner expressions, incremental insert/update/delete plus group removal, HAVING, nested-in-expression,orderByvia$selected, built-in override precedence and restore-on-unregister, unknown-aggregate error content, and non-constant extra argument.🚀 Release Impact
.changeset/smart-pugs-listen.md, minor for@tanstack/db).Summary by CodeRabbit
HAVING, and ordering (including$selected).