feat(mongo-query-ast): add aggregation expression AST#303
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an immutable MongoDB aggregation-expression AST (11 node kinds) with visitor/rewriter support, extends filter AST with an Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Code
participant AggAST as Aggregation\nExpression AST
participant Visitor as Visitor/\nRewriter
participant Lowering as lowerAggExpr
participant Mongo as MongoDB
Client->>AggAST: construct immutable expression nodes
Client->>Visitor: expr.accept(visitor)
Visitor->>Visitor: dispatch node-specific handler
Visitor-->>Client: handler result
Client->>Lowering: lowerAggExpr(expr)
Lowering->>Visitor: traverse via visitor
Visitor-->>Lowering: lowered subexpressions
Lowering->>Lowering: assemble Mongo expression doc
Lowering-->>Mongo: return expression document
sequenceDiagram
participant QueryLayer as Query Layer
participant ExprFilter as MongoExprFilter
participant Lowering as lowerFilter
participant LowerAgg as lowerAggExpr
participant Mongo as MongoDB
QueryLayer->>ExprFilter: create filter with aggExpr
QueryLayer->>Lowering: lowerFilter(filter)
Lowering->>ExprFilter: inspect kind === 'expr'
Lowering->>LowerAgg: lowerAggExpr(aggExpr)
LowerAgg-->>Lowering: expression document
Lowering-->>Mongo: return { $match: { $expr: ... } } or match doc
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
@prisma-next/mongo-orm
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/emitter
@prisma-next/migration-tools
@prisma-next/vite-plugin-contract-emit
@prisma-next/runtime-executor
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-emitter
@prisma-next/mongo-query-ast
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/adapter-postgres
@prisma-next/driver-postgres
commit: |
wmadden
force-pushed
the
tml-2209-aggregation-expression-ast
branch
from
5ed0d2b to
1459b2f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/2-mongo-family/4-query/query-ast/src/aggregation-expressions.ts (1)
116-123: Rewrite always creates new nodes even when unchanged.The
rewrite()method creates a newMongoAggOperatorinstance even when no rewriter hooks modify anything. This is consistent with the other node types and keeps the implementation simple, but for performance-sensitive scenarios with large expression trees, you could consider returningthiswhen children are unchanged (referential equality check).This is fine as-is for correctness; flagging as an optional consideration if performance becomes a concern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/4-query/query-ast/src/aggregation-expressions.ts` around lines 116 - 123, The rewrite method currently always constructs a new MongoAggOperator even when children are unchanged; change rewrite(rewriter: MongoAggExprRewriter) to compute rewrittenArgs (as you already do) then check referential equality: if args is an array ensure every rewrittenArgs[i] === args[i], or if args is not an array check rewrittenArgs === args; in that unchanged case return rewriter.operator ? rewriter.operator(this) : this; otherwise proceed to create const rebuilt = new MongoAggOperator(this.op, rewrittenArgs) and return rewriter.operator ? rewriter.operator(rebuilt) : rebuilt. This uses the existing symbols rewrite, this.args, MongoAggOperator and rewriter.operator.packages/2-mongo-family/4-query/query-ast/test/aggregation-expressions.test.ts (1)
1-621: Comprehensive runtime test coverage.The test suite thoroughly validates:
- Node construction and immutability (Object.isFrozen checks)
- Input validation (empty path rejection)
- Visitor dispatch for all 11 node types
- Rewriter behavior including deep nesting and null-arg preservation
The file exceeds the 500-line guideline at 621 lines. Consider splitting into
aggregation-expressions.nodes.test.ts,aggregation-expressions.visitor.test.ts, andaggregation-expressions.rewriter.test.tsif this grows further, though the current structure is logically cohesive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/4-query/query-ast/test/aggregation-expressions.test.ts` around lines 1 - 621, The test file is 621 lines and exceeds the 500-line guideline; split it into three focused test files: move all construction/immutability and validation tests (describes referencing MongoAggFieldRef, MongoAggLiteral, MongoAggOperator, MongoAggAccumulator, MongoAggCond, MongoAggSwitch, MongoAggArrayFilter, MongoAggMap, MongoAggReduce, MongoAggLet, MongoAggMergeObjects) into aggregation-expressions.nodes.test.ts; move the visitor tests that use the kindVisitor and assertions invoking accept on MongoAggFieldRef, MongoAggLiteral, MongoAggOperator, MongoAggAccumulator, MongoAggCond, MongoAggSwitch, MongoAggArrayFilter, MongoAggMap, MongoAggReduce, MongoAggLet, MongoAggMergeObjects into aggregation-expressions.visitor.test.ts; and move all rewriter tests that reference MongoAggExprRewriter, rewrite calls on MongoAggFieldRef.rewrite, MongoAggOperator.rewrite, MongoAggAccumulator.rewrite, MongoAggCond.rewrite, MongoAggSwitch.rewrite, MongoAggArrayFilter.rewrite, MongoAggMap.rewrite, MongoAggReduce.rewrite, MongoAggLet.rewrite, MongoAggMergeObjects.rewrite into aggregation-expressions.rewriter.test.ts, ensuring each new file includes the same imports (MongoAgg* types and visitors/rewriter types) and that describe/it blocks are preserved unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/3-mongo-target/2-mongo-adapter/src/lowering.ts`:
- Around line 104-111: The needsLiteralWrap function currently only checks
top-level strings and object keys, missing '$' occurrences inside nested
objects/arrays; update needsLiteralWrap to recursively traverse values (handle
arrays, plain objects, and primitive strings) and return true if any encountered
string startsWith('$'), using the existing function name needsLiteralWrap to
locate and replace the logic so that MongoAggLiteral.of([...]) and
MongoAggLiteral.of({label: '$qty'}) are correctly detected and wrapped.
---
Nitpick comments:
In `@packages/2-mongo-family/4-query/query-ast/src/aggregation-expressions.ts`:
- Around line 116-123: The rewrite method currently always constructs a new
MongoAggOperator even when children are unchanged; change rewrite(rewriter:
MongoAggExprRewriter) to compute rewrittenArgs (as you already do) then check
referential equality: if args is an array ensure every rewrittenArgs[i] ===
args[i], or if args is not an array check rewrittenArgs === args; in that
unchanged case return rewriter.operator ? rewriter.operator(this) : this;
otherwise proceed to create const rebuilt = new MongoAggOperator(this.op,
rewrittenArgs) and return rewriter.operator ? rewriter.operator(rebuilt) :
rebuilt. This uses the existing symbols rewrite, this.args, MongoAggOperator and
rewriter.operator.
In
`@packages/2-mongo-family/4-query/query-ast/test/aggregation-expressions.test.ts`:
- Around line 1-621: The test file is 621 lines and exceeds the 500-line
guideline; split it into three focused test files: move all
construction/immutability and validation tests (describes referencing
MongoAggFieldRef, MongoAggLiteral, MongoAggOperator, MongoAggAccumulator,
MongoAggCond, MongoAggSwitch, MongoAggArrayFilter, MongoAggMap, MongoAggReduce,
MongoAggLet, MongoAggMergeObjects) into aggregation-expressions.nodes.test.ts;
move the visitor tests that use the kindVisitor and assertions invoking accept
on MongoAggFieldRef, MongoAggLiteral, MongoAggOperator, MongoAggAccumulator,
MongoAggCond, MongoAggSwitch, MongoAggArrayFilter, MongoAggMap, MongoAggReduce,
MongoAggLet, MongoAggMergeObjects into aggregation-expressions.visitor.test.ts;
and move all rewriter tests that reference MongoAggExprRewriter, rewrite calls
on MongoAggFieldRef.rewrite, MongoAggOperator.rewrite,
MongoAggAccumulator.rewrite, MongoAggCond.rewrite, MongoAggSwitch.rewrite,
MongoAggArrayFilter.rewrite, MongoAggMap.rewrite, MongoAggReduce.rewrite,
MongoAggLet.rewrite, MongoAggMergeObjects.rewrite into
aggregation-expressions.rewriter.test.ts, ensuring each new file includes the
same imports (MongoAgg* types and visitors/rewriter types) and that describe/it
blocks are preserved unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 1ed1c555-8183-4f84-8a25-e6616d9ea702
⛔ Files ignored due to path filters (3)
projects/mongo-pipeline-builder/plan.mdis excluded by!projects/**projects/mongo-pipeline-builder/plans/aggregation-expression-ast-plan.mdis excluded by!projects/**projects/orm-consolidation/plans/aggregation-expression-ast-design.mdis excluded by!projects/**
📒 Files selected for processing (13)
packages/2-mongo-family/4-query/query-ast/src/aggregation-expressions.tspackages/2-mongo-family/4-query/query-ast/src/exports/index.tspackages/2-mongo-family/4-query/query-ast/src/filter-expressions.tspackages/2-mongo-family/4-query/query-ast/src/visitors.tspackages/2-mongo-family/4-query/query-ast/test/aggregation-expressions.test-d.tspackages/2-mongo-family/4-query/query-ast/test/aggregation-expressions.test.tspackages/2-mongo-family/4-query/query-ast/test/filter-expressions.test-d.tspackages/2-mongo-family/4-query/query-ast/test/filter-expressions.test.tspackages/2-mongo-family/4-query/query-ast/vitest.config.tspackages/3-mongo-target/2-mongo-adapter/src/exports/index.tspackages/3-mongo-target/2-mongo-adapter/src/lowering.tspackages/3-mongo-target/2-mongo-adapter/test/lowering.test.tstest/integration/test/mongo/expr-filter.test.ts
wmadden
force-pushed
the
tml-2209-aggregation-expression-ast
branch
from
ec57723 to
2cc70dd
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/3-mongo-target/2-mongo-adapter/src/lowering.ts (2)
124-138: Consider adding exhaustiveness guard tolowerFilterswitch.The switch lacks a
defaultcase orassertNeverguard. If a new filter kind is added toMongoFilterExpr, TypeScript won't error here—the function will silently returnundefined. An exhaustiveness check ensures compile-time safety.♻️ Proposed fix
case 'expr': return { $expr: lowerAggExpr(filter.aggExpr) }; + default: { + const _exhaustive: never = filter; + throw new Error(`Unhandled filter kind: ${(_exhaustive as MongoFilterExpr).kind}`); + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/2-mongo-adapter/src/lowering.ts` around lines 124 - 138, The switch in lowerFilter (function lowerFilter, type MongoFilterExpr) is missing an exhaustiveness guard; add a default path that calls an assertNever-like helper (or throws) to ensure the compiler errors if a new filter kind is added. Implement or reuse a helper such as assertNever(x: never) and invoke it in a final default block (e.g., default: return assertNever(filter)), or explicitly throw an Error including the unknown kind, so lowerFilter never silently returns undefined.
11-15: Minor duplication ofisExprArraytype guard.This function is duplicated from
aggregation-expressions.ts. While the duplication is minor and acceptable for package isolation (query-ast vs adapter), consider extracting to a shared utility if the pattern expands.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/2-mongo-adapter/src/lowering.ts` around lines 11 - 15, The isExprArray type guard is duplicated; to fix, remove the duplicate definition in lowering.ts and instead import the single exported helper from the shared location (e.g., aggregation-expressions.ts or a new utilities module) so the adapter reuses that implementation; update lowering.ts to import isExprArray and delete the local function definition, or create a new shared util (e.g., src/utils/isExprArray) and import it from both aggregation-expressions.ts and lowering.ts to avoid future divergence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/3-mongo-target/2-mongo-adapter/src/lowering.ts`:
- Around line 124-138: The switch in lowerFilter (function lowerFilter, type
MongoFilterExpr) is missing an exhaustiveness guard; add a default path that
calls an assertNever-like helper (or throws) to ensure the compiler errors if a
new filter kind is added. Implement or reuse a helper such as assertNever(x:
never) and invoke it in a final default block (e.g., default: return
assertNever(filter)), or explicitly throw an Error including the unknown kind,
so lowerFilter never silently returns undefined.
- Around line 11-15: The isExprArray type guard is duplicated; to fix, remove
the duplicate definition in lowering.ts and instead import the single exported
helper from the shared location (e.g., aggregation-expressions.ts or a new
utilities module) so the adapter reuses that implementation; update lowering.ts
to import isExprArray and delete the local function definition, or create a new
shared util (e.g., src/utils/isExprArray) and import it from both
aggregation-expressions.ts and lowering.ts to avoid future divergence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: cf115c1b-280d-4e60-a8d9-cf6eb6ffd2a1
⛔ Files ignored due to path filters (3)
projects/mongo-pipeline-builder/plan.mdis excluded by!projects/**projects/mongo-pipeline-builder/plans/aggregation-expression-ast-plan.mdis excluded by!projects/**projects/orm-consolidation/plans/aggregation-expression-ast-design.mdis excluded by!projects/**
📒 Files selected for processing (13)
packages/2-mongo-family/4-query/query-ast/src/aggregation-expressions.tspackages/2-mongo-family/4-query/query-ast/src/exports/index.tspackages/2-mongo-family/4-query/query-ast/src/filter-expressions.tspackages/2-mongo-family/4-query/query-ast/src/visitors.tspackages/2-mongo-family/4-query/query-ast/test/aggregation-expressions.test-d.tspackages/2-mongo-family/4-query/query-ast/test/aggregation-expressions.test.tspackages/2-mongo-family/4-query/query-ast/test/filter-expressions.test-d.tspackages/2-mongo-family/4-query/query-ast/test/filter-expressions.test.tspackages/2-mongo-family/4-query/query-ast/vitest.config.tspackages/3-mongo-target/2-mongo-adapter/src/exports/index.tspackages/3-mongo-target/2-mongo-adapter/src/lowering.tspackages/3-mongo-target/2-mongo-adapter/test/lowering.test.tstest/integration/test/mongo/expr-filter.test.ts
✅ Files skipped from review due to trivial changes (5)
- packages/2-mongo-family/4-query/query-ast/vitest.config.ts
- packages/3-mongo-target/2-mongo-adapter/src/exports/index.ts
- test/integration/test/mongo/expr-filter.test.ts
- packages/2-mongo-family/4-query/query-ast/test/aggregation-expressions.test-d.ts
- packages/2-mongo-family/4-query/query-ast/src/exports/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/2-mongo-family/4-query/query-ast/test/filter-expressions.test-d.ts
- packages/2-mongo-family/4-query/query-ast/src/filter-expressions.ts
- packages/3-mongo-target/2-mongo-adapter/test/lowering.test.ts
- packages/2-mongo-family/4-query/query-ast/src/visitors.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/3-mongo-target/2-mongo-adapter/test/lowering.test.ts`:
- Around line 335-377: The tests assert single-$ field refs for scoped variables
but Mongo requires $$ for vars created by $filter/$map/$reduce/$let; update the
lowering so lowerAggExpr becomes context-aware: when descending into
MongoAggArrayFilter (MongoAggArrayFilter.of), MongoAggMap.of, MongoAggReduce.of
and any MongoAggLet node, pass the scoped-variable name (the "as" param or
accumulator name) and when lowering MongoAggFieldRef resolve to a scoped var
reference by prefixing its root with '$$' (e.g., 'score' -> '$$score',
'item.price' -> '$$item.price', 'value'/'this' -> '$$value'/'$$this') instead of
single '$'; adjust test expectations accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 0d6004d0-abe7-40bd-a3f3-7f5f5c787d27
📒 Files selected for processing (2)
packages/3-mongo-target/2-mongo-adapter/src/lowering.tspackages/3-mongo-target/2-mongo-adapter/test/lowering.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/3-mongo-target/2-mongo-adapter/src/lowering.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/3-mongo-target/2-mongo-adapter/test/lowering.test.ts (1)
292-333: Simplify$condand$switchtest assertions.The
Object.fromEntrieswith athenKeyvariable adds unnecessary indirection. The keywordthenis valid in object literals and doesn't conflict with Promise semantics in this context.♻️ Proposed simplification
it('lowers $cond', () => { const expr = MongoAggCond.of( MongoAggOperator.of('$gte', [MongoAggFieldRef.of('age'), MongoAggLiteral.of(18)]), MongoAggLiteral.of('adult'), MongoAggLiteral.of('minor'), ); - const thenKey = 'then'; expect(lowerAggExpr(expr)).toEqual({ - $cond: Object.fromEntries([ - ['if', { $gte: ['$age', 18] }], - [thenKey, 'adult'], - ['else', 'minor'], - ]), + $cond: { + if: { $gte: ['$age', 18] }, + then: 'adult', + else: 'minor', + }, }); }); it('lowers $switch', () => { const expr = MongoAggSwitch.of( [ { case_: MongoAggOperator.of('$eq', [ MongoAggFieldRef.of('status'), MongoAggLiteral.of('active'), ]), then_: MongoAggLiteral.of('Active'), }, ], MongoAggLiteral.of('Unknown'), ); - const thenKey = 'then'; expect(lowerAggExpr(expr)).toEqual({ $switch: { - branches: [ - Object.fromEntries([ - ['case', { $eq: ['$status', 'active'] }], - [thenKey, 'Active'], - ]), - ], + branches: [{ case: { $eq: ['$status', 'active'] }, then: 'Active' }], default: 'Unknown', }, }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/2-mongo-adapter/test/lowering.test.ts` around lines 292 - 333, The tests for lowering $cond and $switch use an unnecessary indirection with Object.fromEntries and a thenKey variable; update the assertions in lowering.test.ts to use direct object literals with the explicit "then" property (remove thenKey and Object.fromEntries) so the expected outputs for lowerAggExpr(MongoAggCond.of(...)) and lowerAggExpr(MongoAggSwitch.of(...)) assert { $cond: { if: ..., then: 'adult', else: 'minor' } } and { $switch: { branches: [{ case: ..., then: 'Active' }], default: 'Unknown' } } respectively, leaving the MongoAggCond, MongoAggSwitch, and lowerAggExpr identifiers unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/3-mongo-target/2-mongo-adapter/test/lowering.test.ts`:
- Around line 292-333: The tests for lowering $cond and $switch use an
unnecessary indirection with Object.fromEntries and a thenKey variable; update
the assertions in lowering.test.ts to use direct object literals with the
explicit "then" property (remove thenKey and Object.fromEntries) so the expected
outputs for lowerAggExpr(MongoAggCond.of(...)) and
lowerAggExpr(MongoAggSwitch.of(...)) assert { $cond: { if: ..., then: 'adult',
else: 'minor' } } and { $switch: { branches: [{ case: ..., then: 'Active' }],
default: 'Unknown' } } respectively, leaving the MongoAggCond, MongoAggSwitch,
and lowerAggExpr identifiers unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 4c0c03d5-925d-4474-ae42-be9558ef41e3
📒 Files selected for processing (2)
packages/2-mongo-family/4-query/query-ast/test/aggregation-expressions.test.tspackages/3-mongo-target/2-mongo-adapter/test/lowering.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/2-mongo-family/4-query/query-ast/test/aggregation-expressions.test.ts
Defines the implementation approach for the MongoAggExpr class hierarchy in @prisma-next/mongo-query-ast: 11 concrete expression node classes, visitor/rewriter interfaces, lowerAggExpr() lowering, and the MongoExprFilter bridge for $expr in $match.
Add dedicated type test section verifying union exhaustiveness, visitor exhaustiveness, rewriter optionality, and structural type constraints. Add integration test section for $expr cross-field comparisons against mongodb-memory-server. Include test file inventory and note that $group integration tests follow in Milestone 3.
Introduce 11 aggregation expression AST node classes in @prisma-next/mongo-query-ast: MongoAggFieldRef, MongoAggLiteral, MongoAggOperator, MongoAggAccumulator, MongoAggCond, MongoAggSwitch, MongoAggArrayFilter, MongoAggMap, MongoAggReduce, MongoAggLet, and MongoAggMergeObjects. Add MongoAggExprVisitor<R> (exhaustive) and MongoAggExprRewriter (optional hooks) interfaces to visitors.ts. All node classes implement accept() and rewrite() following the established filter expression pattern (bottom-up rewriting for container nodes). 72 unit tests cover construction, freezing, kind discriminants, static helpers, visitor dispatch, and rewriter transforms including deep nesting.
Verify compile-time guarantees: MongoAggExpr union exhaustiveness (all 11 kinds), MongoAggExprVisitor requires all methods, rewriter accepts empty object, accept() returns R, rewrite() returns the union type, and structural constraints on args/branches/vars. Enable vitest typecheck in query-ast vitest config. Fix MongoAggOperator.rewrite() narrowing with an explicit type guard for ReadonlyArray vs single-expression args.
Implement lowerAggExpr() as a MongoAggExprVisitor<unknown> in the
adapter lowering module. Handles all 11 node types: field refs get
$-prefix, literals pass through (with $literal wrapping for
ambiguous values), operators preserve single-arg vs array-arg form,
accumulators emit {} for $count, and structural nodes produce their
specific MongoDB shapes with recursively lowered children.
Export all MongoAggExpr types and visitor/rewriter interfaces from
@prisma-next/mongo-query-ast. Export lowerAggExpr from
@prisma-next/adapter-mongo.
Introduce MongoExprFilter, a filter expression node that wraps a MongoAggExpr to enable aggregation expressions inside $match stages via the $expr operator. This bridges the two expression systems, enabling cross-field comparisons and computed predicates in filters. Add expr() to MongoFilterVisitor and MongoFilterRewriter interfaces. Update lowerFilter() to handle the new expr kind. Export MongoExprFilter from @prisma-next/mongo-query-ast. Add unit tests, type tests, and lowering tests.
Verify end-to-end execution of MongoExprFilter against a live MongoDB instance: cross-field comparison ($gt with two field refs), computed expression with nested arithmetic ($multiply inside $gt), and $expr combined with a regular MongoFieldFilter via $and.
Reconcile MongoAggFilter→MongoAggArrayFilter naming across plan, design, and parent plan docs (NB-F02). Add Biome noThenProperty rationale comment on THEN_KEY/condBranch (NB-F03). Update design doc to reflect the intentional unknown (not MongoValue) type for MongoAggLiteral.value (NB-F04). Add empty-path guard to MongoAggFieldRef constructor with matching test (NB-F05).
describeWithMongoDB drops the database in beforeEach, so tests sharing a collection name are isolated. Make this visible at the test site.
Array.isArray does not narrow ReadonlyArray out of a union with non-array types. Use the same isExprArray type guard pattern as in aggregation-expressions.ts.
…ss guard
needsLiteralWrap now recursively checks arrays and object values for
$-prefixed strings, not just top-level keys. Previously, literals like
["$qty"] or {label: "$qty"} would be emitted without $literal wrapping,
causing MongoDB to misinterpret them as field references.
Also adds a never-typed default case to lowerFilter switch for
compile-time exhaustiveness.
MongoDB scoped variables ($filter as, $map as, $reduce $$value/$$this,
$let vars) require $$ prefix. MongoAggFieldRef.of("$score") lowers to
"$$score" since lowering prepends $. Updated tests to use this convention
so lowered output matches correct MongoDB semantics.
wmadden
force-pushed
the
tml-2209-aggregation-expression-ast
branch
from
2774ff1 to
d1c8289
Compare
This was referenced Apr 7, 2026
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.
closes TML-2209
Key snippet (new capability)
Intent
Add a typed representation of MongoDB aggregation expressions — the expression language used inside pipeline stages like
$group,$project, and$addFields— so that the pipeline builder (Milestone 4) has a foundation to construct computed values, and so that cross-field comparisons become possible via the$exprbridge into filter context.Change map
Implementation:
MongoAggExprunionMongoAggExprVisitor<R>andMongoAggExprRewriterinterfaces;MongoFilterVisitor/MongoFilterRewriterextended withexprMongoExprFilterbridge class;MongoFilterExprunion extendedlowerAggExpr()visitor,needsLiteralWrap(),condBranch()helper;lowerFilter()extended withcase 'expr'lowerAggExprexportTests (evidence):
MongoExprFilterconstruction, freezing, visitor dispatch, rewriterMongoFilterExprunion includes'expr',MongoFilterVisitorrequiresexprmethod$exprfilter lowering$exprexecution against mongodb-memory-serverThe story
Introduce the aggregation expression node classes. MongoDB's aggregation pipeline needs a way to represent computed values — field references (
"$name"), arithmetic ({ $add: [...] }), conditionals ({ $cond: {...} }), array transforms ({ $map: {...} }), and more. Eleven concrete classes model the distinct structural shapes of MongoDB aggregation expressions, following the same patterns as the existing filter expression AST: hidden abstract base,kinddiscriminant, immutable frozen instances, static factories.Add visitor and rewriter interfaces. The exhaustive
MongoAggExprVisitor<R>ensures every expression kind is handled (compile-time enforcement). The partialMongoAggExprRewriterenables selective bottom-up transformations — rewrite children first, then apply the hook for the current node. Both interfaces are wired intoaccept()andrewrite()on every node class.Implement lowering to MongoDB driver documents.
lowerAggExpr()translates the typed AST into plain objects the MongoDB driver expects. Key behaviors: field refs get$-prefixed, ambiguous literals are wrapped in{ $literal: ... }, operators dispatch between single-arg and array-arg forms, and the$countaccumulator emits{}.Bridge aggregation expressions into filter context via
MongoExprFilter. The$exproperator in MongoDB allows aggregation expressions inside$match— this is the only way to do cross-field comparisons in a filter.MongoExprFilterwraps aMongoAggExprand joins theMongoFilterExprunion. The filter rewriter treats the inner aggregation expression as opaque (intentional: the two rewriter systems are independent).Validate end-to-end with integration tests. Three integration tests execute
$expr-based filters through the full runtime pipeline against mongodb-memory-server: a simple cross-field comparison (qty > minQty), nested arithmetic (price > discount * 2), and a combined$andwith both regular and$exprfilters.Behavior changes & evidence
Adds a typed aggregation expression AST (
MongoAggExpr) covering 11 expression shapes. Field references, literals, uniform operators, accumulators, conditionals, switch, array filter, map, reduce, let, and merge-objects — the full set of structurally distinct aggregation expression forms in MongoDB's pipeline language.$group,$project, and$addFieldsrequire. Without this layer, those stages cannot be represented as typed nodes.Adds
lowerAggExpr()— lowering aggregation expression AST to MongoDB driver documents. Each AST node maps to its MongoDB wire format. IncludesneedsLiteralWrap()for detecting$-prefixed strings and objects that need{ $literal: ... }escaping.Extends
MongoFilterExprunion withMongoExprFilterfor$exprsupport. Filter expressions can now embed aggregation expressions for cross-field comparisons.MongoFilterVisitor<R>gains a requiredexpr()method;MongoFilterRewritergains an optionalexpr?()hook.$exproperator is the only way to compare two fields in a filter context ({ $match: { $expr: { $gt: ["$qty", "$minQty"] } } }). Without this bridge, cross-field comparisons are impossible in the typed AST.Compatibility / migration / risk
MongoFilterVisitor<R>gains a requiredexpr()method. Any existing implementations ofMongoFilterVisitor(in-repo or external) must add this method. This is a compile-time breaking change for visitor consumers. Within this repo, all consumers are updated in this branch.MongoFilterExprunion is extended. Exhaustive switches onMongoFilterExpr.kindmust add acase 'expr'arm. This is also a compile-time breaking change for exhaustive consumers.lowerFilter()is extended. The newcase 'expr'arm callslowerAggExpr(), introducing a runtime dependency between filter lowering and aggregation expression lowering. This is a forward-only change with no backward compatibility concern.No regressions in existing filter, stage, or command behavior. The changes are additive.
Follow-ups / open questions
MongoAggAccumulatornodes are unit-tested and lowering-tested, but cannot be integration-tested untilMongoGroupStageexists (Milestone 3).MongoAggFilterbut the implementation usesMongoAggArrayFilter. The plan should be updated.MongoAggLiteral.valuetype: Design doc saysMongoValue, implementation usesunknown. A decision should be recorded.Non-goals / intentionally out of scope
MongoGroupStage,MongoAddFieldsStage, etc.) — Milestone 3Summary by CodeRabbit