From e6a1e2924d4acb5bb41b0478df1bbbe7171498a9 Mon Sep 17 00:00:00 2001 From: Jaime Resano Date: Thu, 1 Oct 2026 15:04:48 +0200 Subject: [PATCH 1/4] fix(db): keep residual clauses residual when combining WHERE clauses applyOptimizations combined the remaining WHERE clauses of an outer-join query into one plain AND, dropping the residual marker on predicates that were already pushed into a subquery. The next optimization pass treated them as new predicates and pushed them again, so optimizeQuery only stopped at its iteration limit and left duplicated predicates in the subquery. Co-Authored-By: Claude Opus 5.5 --- .changeset/optimizer-residual-convergence.md | 5 +++ packages/db/src/query/optimizer.ts | 22 +++++++++-- packages/db/tests/query/optimizer.test.ts | 39 ++++++++++++++++++++ 3 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 .changeset/optimizer-residual-convergence.md diff --git a/.changeset/optimizer-residual-convergence.md b/.changeset/optimizer-residual-convergence.md new file mode 100644 index 0000000000..981e03e907 --- /dev/null +++ b/.changeset/optimizer-residual-convergence.md @@ -0,0 +1,5 @@ +--- +'@tanstack/db': patch +--- + +Keep residual WHERE clauses marked as residual when the optimizer combines the remaining clauses of an outer-join query, so predicates pushed into a subquery are not pushed again on every pass until the iteration limit. diff --git a/packages/db/src/query/optimizer.ts b/packages/db/src/query/optimizer.ts index b7487533ed..8062ab0016 100644 --- a/packages/db/src/query/optimizer.ts +++ b/packages/db/src/query/optimizer.ts @@ -786,16 +786,30 @@ function applyOptimizations( // Combine multiple remaining WHERE clauses into a single clause to avoid // multiple filter operations in the pipeline (performance optimization) // First flatten any nested AND expressions to avoid and(and(...), ...) - const finalWhere: Array = - remainingWhereClauses.length > 1 + // Residual clauses are combined separately: merging them into a regular + // clause would drop the residual marker, so the next optimization pass + // would push the same predicates down again and never converge. + const combineRemaining = (clauses: Array): Array => + clauses.length > 1 ? [ combineWithAnd( - remainingWhereClauses.flatMap((clause) => + clauses.flatMap((clause) => splitAndClausesRecursive(getWhereExpression(clause)), ), ), ] - : remainingWhereClauses + : clauses + const residualClauses = remainingWhereClauses.filter(isResidualWhere) + const finalWhere: Array = [ + ...combineRemaining( + remainingWhereClauses.filter((clause) => !isResidualWhere(clause)), + ), + ...(residualClauses.length > 1 + ? combineRemaining(residualClauses).map((clause) => + createResidualWhere(getWhereExpression(clause)), + ) + : residualClauses), + ] // Preserve untouched query options while replacing the optimized clauses. const optimizedQuery: QueryIR = { diff --git a/packages/db/tests/query/optimizer.test.ts b/packages/db/tests/query/optimizer.test.ts index 52e4367c01..babeabca93 100644 --- a/packages/db/tests/query/optimizer.test.ts +++ b/packages/db/tests/query/optimizer.test.ts @@ -7,6 +7,7 @@ import { PropRef, QueryRef, Value, + createResidualWhere, } from '../../src/query/ir.js' import type { QueryIR } from '../../src/query/ir.js' @@ -1697,6 +1698,44 @@ describe(`Query Optimizer`, () => { expect(optimizedQuery.join![0]!.from.type).toBe(`collectionRef`) }) + test(`should push a LEFT JOIN active-side clause down once when a nullable-side clause remains`, () => { + const teamsCollection = { id: `teams` } as any + const teamMembersCollection = { id: `team-members` } as any + + const memberFilter = createEq( + createPropRef(`teamMember`, `user_id`), + createValue(100), + ) + const teamFilter = createEq( + createPropRef(`team`, `active`), + createValue(true), + ) + + const query: QueryIR = { + from: new CollectionRef(teamsCollection, `team`), + join: [ + { + type: `left`, + from: new CollectionRef(teamMembersCollection, `teamMember`), + left: createPropRef(`team`, `id`), + right: createPropRef(`teamMember`, `team_id`), + }, + ], + where: [memberFilter, teamFilter], + } + + const { optimizedQuery } = optimizeQuery(query) + + expect(optimizedQuery.from.type).toBe(`queryRef`) + expect((optimizedQuery.from as QueryRef).query.where).toEqual([ + teamFilter, + ]) + expect(optimizedQuery.where).toEqual([ + memberFilter, + createResidualWhere(teamFilter), + ]) + }) + test(`should preserve WHERE clause semantics when pushing down to RIGHT JOIN`, () => { // This test reproduces the bug where pushing WHERE clauses into RIGHT JOIN subqueries // changes the semantics by filtering out null values that should remain From 410239862af7cbab8132fafd800c15957d8a7266 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 1 Oct 2026 10:51:01 -0600 Subject: [PATCH 2/4] fix(db): keep residual predicates stable across passes --- docs/contributing/oracle-coverage.md | 2 +- ...2026-10-01-pr-1977-residual-convergence.md | 82 +++ packages/db/src/query/optimizer.ts | 56 +- .../query/optimizer-semantics-oracle.test.ts | 533 ++++++++++++++++++ 4 files changed, 633 insertions(+), 40 deletions(-) create mode 100644 docs/contributing/oracle-reviews/2026-10-01-pr-1977-residual-convergence.md diff --git a/docs/contributing/oracle-coverage.md b/docs/contributing/oracle-coverage.md index d66b3d3d7e..da025b9c88 100644 --- a/docs/contributing/oracle-coverage.md +++ b/docs/contributing/oracle-coverage.md @@ -249,7 +249,7 @@ comment and the current API/architecture contract before extending its model. | Lazy target path identity | `packages/db/tests/query/compiler/lazy-targets.test.ts` | A focused same-source `UnionFrom`/`coalesce` witness requires both [`a.b`] and [`a`, `b`] demand targets. Restoring dotted-string deduplication drops the second target at the compiler boundary. A public on-demand adapter and row-publication history still need a separate witness. | | Correlated include path identity | `packages/db/tests/query/includes-context-transport-oracle.test.ts` | A flat parent field and nested parent field reach one-level and nested `toArray` results independently; dotted ancestor aliases remain distinct through a grandchild route. Initial results and parent updates have exact public-value checks. A conditional projection gives [`a.b`] and [`a`, `b`] different include results and checks parent-key changes plus later child inserts. Removing the compiler's unique route-key allocator loses a public child result; restoring dotted-string deduplication at either builder site loses a distinct parent value. These fixed witnesses do not establish arbitrary path segments, every recursive source form, or every materialization form. | | Join equality and cold acquisition | `packages/db/tests/query/cold-join-reconciliation-oracle.test.ts` | Independent recomputation for cold acquisition plus direct join/predicate equivalence across established equality domains. Binary/string and nullish classes, replacement histories, raw on-demand values, and both scan/auto-index paths are explicit; compound join syntax is not claimed. | -| Optimizer aggregate pushdown | `packages/db/tests/query/optimizer-semantics-oracle.test.ts`, [review record](oracle-reviews/2026-09-28-optimizer-aggregate-pushdown.md), [repair record](oracle-reviews/2026-09-30-queryref-publication-repair.md) | Independent sum recomputation and a materialized-Collection formulation check the first public snapshot of a nested aggregate under a left join. Direct, arithmetic-wrapped, and conditional aggregates distinguish safe from unsafe pushdown; a grouped-key control covers the grouped boundary. Matching and nonmatching global aggregates also cross a nested QueryRef inner join with absent, accepting, and rejecting outer predicates. The matching initial cases fail on the pre-repair revision and pass after the computed-projection lookup repair. Other predicates, wrappers, join forms, and incremental optimizer histories remain outside this owner. | +| Query optimizer semantics | `packages/db/tests/query/optimizer-semantics-oracle.test.ts`, [aggregate review](oracle-reviews/2026-09-28-optimizer-aggregate-pushdown.md), [QueryRef repair](oracle-reviews/2026-09-30-queryref-publication-repair.md), [residual review](oracle-reviews/2026-10-01-pr-1977-residual-convergence.md) | Independent sum recomputation and a materialized-Collection formulation check the first public snapshot of a nested aggregate under a left join. Direct, arithmetic-wrapped, and conditional aggregates distinguish safe from unsafe pushdown; a grouped-key control covers the grouped boundary. Matching and nonmatching global aggregates also cross a nested QueryRef inner join with absent, accepting, and rejecting outer predicates. The matching initial cases fail on the pre-repair revision and pass after the computed-projection lookup repair. A separate fixed residual-convergence grammar checks exact predicate and outer WHERE clause counts at `optimizeQuery` return and after re-optimization: LEFT, RIGHT, INNER, and FULL joins; equality, greater-than, nullable-field `isUndefined`, same-source OR, cross-source equality, clause order, separate/AND form, two active predicates, two residual sources, and an existing residual marker. Plain-array LEFT/RIGHT and three-source join models check first public rows, including unmatched rows. Arbitrary expressions, more complex join chains, and incremental optimizer/publication histories remain outside this owner. | | QueryRef operators and user-value boundaries | `packages/db/tests/query/subquery-user-value-oracle.test.ts`, `packages/db/tests/transactions.test.ts`, [repair record](oracle-reviews/2026-09-30-queryref-publication-repair.md) | A finite array/Set model checks the compiled output bag for a joined DISTINCT subquery with an outer WHERE. Live-query drivers check selected containers, no-select, functional, and predicate-literal user objects with IR-like fields, top-level and aggregate-subquery proxy-shaped fields, arrays of selected references, outer virtual-field filters on joined DISTINCT and nested aggregate QueryRefs, and a renamed no-select source. The transaction suite documents the existing development-browser duplicate-load guard. The bounded cases run in `test:oracles` or the full DB suite. Joined DISTINCT, nested and flat aggregate source-update histories pass. Ordered joined `findOne()` checks initial, singleton, empty, restored, and changed-join-key cuts; an unordered `findOne()` QueryRef on either side of a join checks default-key candidate deletion and reinsertion. A materialized joined `findOne()` supplies a separate receiving formulation. Other `singleResult` forms, join forms, schedules, and cross-copy value handling outside the duplicate-load guard remain unproved. | | Opaque backend pagination | [window oracle](https://github.com/TanStack/db/blob/main/packages/query-db-collection/tests/cursor-pagination.oracle.test.ts), [cache histories](https://github.com/TanStack/db/blob/main/packages/query-db-collection/tests/cursor-pagination.cache-oracle.test.ts), [cache publication](https://github.com/TanStack/db/blob/main/packages/query-db-collection/tests/cursor-pagination.publication-oracle.test.ts), [browser acquisition boundaries](https://github.com/TanStack/db/blob/main/packages/query-db-collection/tests/cursor-pagination.boundary-oracle.test.ts), [QueryCollection integration](https://github.com/TanStack/db/blob/main/packages/query-db-collection/tests/cursor-pagination.integration.test.ts) | Full filter/sort/slice reference, opaque token transport, actual Query cache expiry/invalidation/GC, forced refresh during growth, protocol failure publication/recovery, bounded slice work, nested cancellation/replacement, reader abort, browser retry defaults, manual-write cache isolation, and production window publications. Stable backend sequences; not snapshot guarantees for changing endpoints. Peek-ahead remains enabled. | | Electric and TrailBase | [Electric histories](https://github.com/TanStack/db/blob/main/packages/electric-db-collection/tests/electric-oracle.property.test.ts), [recovery histories](https://github.com/TanStack/db/blob/main/packages/electric-db-collection/tests/electric-recovery-oracle.test.ts), [held resume snapshots](https://github.com/TanStack/db/blob/main/packages/electric-db-collection/tests/electric-resume-snapshot-races.test.ts), [PostgreSQL semantics](https://github.com/TanStack/db/blob/main/packages/electric-db-collection/e2e/sql-predicate-semantics.e2e.test.ts), [TrailBase contract](https://github.com/TanStack/db/blob/main/packages/trailbase-db-collection/tests/ORACLE.md) | Installed SDK delivery/framing, independent predicates, exact subscription arguments, restart/reset lineage, held certification and durability races, source-order publication before durability, and late errors. The queued-presence property runs identical fixed/random generators plus isolated seed-and-path replay across insert, update, delete, and truncate callbacks. The recovery fixtures use a mocked ShapeStream; they do not establish live Electric-service framing or native persistence-host behavior. | diff --git a/docs/contributing/oracle-reviews/2026-10-01-pr-1977-residual-convergence.md b/docs/contributing/oracle-reviews/2026-10-01-pr-1977-residual-convergence.md new file mode 100644 index 0000000000..7dd7f5a580 --- /dev/null +++ b/docs/contributing/oracle-reviews/2026-10-01-pr-1977-residual-convergence.md @@ -0,0 +1,82 @@ +# PR 1977 residual-convergence oracle review + +The reviewed baseline is origin/main at +`18abceee48ebde712e120cbc541289bb83f35d77`. The contributor's supplied +patch corresponds to proposed head `e6a1e2924d4acb5bb41b0478df1bbbe7171498a9`. +The reviewed candidate is that baseline plus the uncommitted changes whose +optimizer file has SHA-256 +`fee8078ebdfc40d9a431a7c48f7061dd1afbc0f7383a95a13f7d87507248bfc3` +and oracle file has SHA-256 +`bd5b06605814d3f1c190455438974f9112a53219930ec7d8b92673917d3dc56c`. +The separate task-local external-feedback ledger preserves all 13 PR and bot +items; this record preserves the oracle and repair evidence. + +## Contract and checked boundary + +The optimizer's predicate-pushdown contract permits a predicate on a +nonnullable outer-join source to enter that source. The joined row still needs +the predicate, so its outer copy is residual and must not be pushed again. +A predicate on a nullable source stays regular. One logical predicate should +enter each eligible source once, including when optimization runs again. +The public-row contract requires the same result as applying predicates after +the join. These rules follow the optimizer's documented pushdown behavior and +the established outer-join tests. + +The finite structural grammar crosses LEFT, RIGHT, INNER, and FULL joins with +separate or AND-combined clauses, both clause orders, and one or two active +predicates. Fixed extensions cover a LEFT/INNER chain with two pushed sources, +same-source OR, cross-source equality, greater-than, nullable-field +`isUndefined`, and an already residual outer clause. The checkpoint is the +returned `optimizeQuery` IR, both initially and after re-optimization. +The oracle counts each predicate in its source and outer WHERE and checks the +outer residual marker and clause count. Plain-array LEFT, RIGHT, and +three-source join models separately check the first public snapshot of a +live-query Collection, with matched and unmatched source rows. + +## RED, GREEN, and code weight + +| Subject | Result at the intended checkpoint | +| --- | --- | +| Original optimizer, full expanded oracle | 21 failed, 28 passed. Source copies multiply and outer residual markers disappear; the new greater-than witness alone fails at source predicate multiplicity. | +| Contributor's supplied patch, full expanded oracle | 1 failed, 48 passed. A later pass with an existing residual leaves three outer WHERE clauses, while the law expects one regular and one residual group. Predicate multiplicities and markers pass that case. | +| Candidate partition-and-combine repair, full expanded oracle | 49 passed. The candidate combines each regular/residual group once inside `applyOptimizations` and removes caller-side residual reassembly. | +| Candidate, optimizer, join, and join-subquery suites | 247 passed after the predicate-kind case was added. | +| Wrong-design controls | Marking regular clauses residual fails on `member.userId`'s marker. Keeping only the first residual fails on the missing `tag.region` predicate. Both are assertion failures at the structural checkpoint, not setup failures or timeouts. | + +The production change adds 20 lines and removes 28, net **minus eight**. +The expanded executable oracle adds 533 lines. This separates the cost of the +repair from the broader test and documentation coverage. + +## Oracle guide audit + +| ID | Outcome | +| --- | --- | +| ORC-001 | The contract above states the optimizer and outer-join law, authority, finite grammar, and observations. The coverage map owns omitted expressions, join chains, and publication histories. | +| ORC-002 | The structural expectation is an explicit relational-side table, not the optimizer's nullable-source classifier. The public result models join and filter plain arrays; they import neither optimizer analysis nor compiler evaluation. | +| ORC-003 | The oracle opening states the contract, model, grammar, driver, and checkpoints. The table and array models give the answer; `optimizeQuery` and `createLiveQueryCollection` are the production drivers; assertions are adjacent to each case. | +| ORC-004 | Not applicable: this is a finite, enumerated grammar, not a generated-history property. Its named cross product and fixed extensions are the claim's bound. | +| ORC-005 | Structural work is observed at the `optimizeQuery` return and after a later pass. The public driver observes exact projected rows at the first synchronous Collection snapshot. The recorder preserves duplicate predicate terms and result rows. | +| ORC-006 | The original implementation and two wrong designs fail at the intended assertions. The contributor's partial fix also fails the later-pass clause-count assertion. | +| ORC-007 | Not applicable: there is no important generated property or random campaign. Every enumerated case runs in the package oracle test file. | +| ORC-008 | Not applicable: the structural table and public array models are stateless; no model state is merged or split. | +| ORC-009 | Source Collection, live-query Collection, public row, public snapshot, and checkpoint use the glossary meanings. The model's `teamPush` and `memberPush` booleans describe expected source placement, not production states. | +| ORC-010 | Not applicable: the fixed tests do not shrink, record mutable traces, or perform cleanup that could replace an assertion failure. | +| ORC-011 | The plain-array public result is a different formulation from the structural predicate-count law. It checks the plausible shared fault that a plan with fewer predicates changed joined rows, including unmatched rows. It does not independently prove optimizer work cost. | +| ORC-012 | This record scores every applicable requirement and identifies non-applicable triggers. The closure claim is limited to the stated grammar, paths, and checkpoints; remaining cells are assigned in the coverage map. | +| ORC-013 | The original ten-copy result and the repaired one-copy result distinguish the once-only law from repeated pushdown. LEFT and RIGHT active-side cases distinguish the relational-side boundary from a LEFT-only rule; FULL is the no-push control. The existing-residual case rejects a plausible partial grouping. | +| ORC-014 | Not applicable to the structural path, which has no controlled provider. The public driver uses synchronous in-memory source Collections and claims only their first snapshot; it does not claim an external-provider handoff. | + +This closes the reported repeated-pushdown class for the finite structural +grammar and initial public-result paths above. Arbitrary expression trees, +more complex join chains, and incremental source changes or publication +histories remain open cells for the optimizer-semantics owner. The current +tests give no wall-clock runtime guarantee. + +## Package-wide validation + +After building the isolated candidate package, the parent audit ran +`pnpm exec tsc --noEmit -p packages/db/tsconfig.json --pretty false` +successfully. The full `packages/db` Vitest suite then passed 229 files and +7,937 tests with no type errors using `--testTimeout=120000`. This resolves +the earlier self-package declaration setup errors. The structural and public +history bounds above remain unchanged. diff --git a/packages/db/src/query/optimizer.ts b/packages/db/src/query/optimizer.ts index 8062ab0016..7d8ecba120 100644 --- a/packages/db/src/query/optimizer.ts +++ b/packages/db/src/query/optimizer.ts @@ -429,20 +429,7 @@ function applySingleLevelOptimization(query: QueryIR): QueryIR { const groupedClauses = groupWhereClauses(analyzedClauses) // Step 4: Apply optimizations by lifting single-source clauses into subqueries - const optimizedQuery = applyOptimizations(query, groupedClauses) - - // Add back any residual WHERE clauses that were filtered out - const residualWhereClauses = query.where.filter((where) => - isResidualWhere(where), - ) - if (residualWhereClauses.length > 0) { - optimizedQuery.where = [ - ...(optimizedQuery.where || []), - ...residualWhereClauses, - ] - } - - return optimizedQuery + return applyOptimizations(query, groupedClauses) } /** @@ -760,8 +747,9 @@ function applyOptimizations( })) : undefined - // Build the remaining WHERE clauses: multi-source + residual single-source clauses + // Keep regular and residual clauses separate so neither loses its marker. const remainingWhereClauses: Array = [] + const residualWhereClauses = query.where?.filter(isResidualWhere) ?? [] // Add multi-source clauses if (groupedClauses.multiSource) { @@ -778,37 +766,27 @@ function applyOptimizations( remainingWhereClauses.push(clause) } else if (hasOuterJoins) { // Was optimized AND query has outer JOINs - keep as residual WHERE clause - remainingWhereClauses.push(createResidualWhere(clause)) + residualWhereClauses.push(createResidualWhere(clause)) } // If optimized and no outer JOINs - don't keep (original behavior) } - // Combine multiple remaining WHERE clauses into a single clause to avoid - // multiple filter operations in the pipeline (performance optimization) - // First flatten any nested AND expressions to avoid and(and(...), ...) - // Residual clauses are combined separately: merging them into a regular - // clause would drop the residual marker, so the next optimization pass - // would push the same predicates down again and never converge. - const combineRemaining = (clauses: Array): Array => + // Combine within each group to avoid extra filters and nested ANDs. + const combineRemaining = (clauses: Array): Where | undefined => clauses.length > 1 - ? [ - combineWithAnd( - clauses.flatMap((clause) => - splitAndClausesRecursive(getWhereExpression(clause)), - ), + ? combineWithAnd( + clauses.flatMap((clause) => + splitAndClausesRecursive(getWhereExpression(clause)), ), - ] - : clauses - const residualClauses = remainingWhereClauses.filter(isResidualWhere) - const finalWhere: Array = [ - ...combineRemaining( - remainingWhereClauses.filter((clause) => !isResidualWhere(clause)), - ), - ...(residualClauses.length > 1 - ? combineRemaining(residualClauses).map((clause) => - createResidualWhere(getWhereExpression(clause)), ) - : residualClauses), + : clauses[0] + const regularWhere = combineRemaining(remainingWhereClauses) + const residualWhere = combineRemaining(residualWhereClauses) + const finalWhere: Array = [ + ...(regularWhere ? [regularWhere] : []), + ...(residualWhere + ? [createResidualWhere(getWhereExpression(residualWhere))] + : []), ] // Preserve untouched query options while replacing the optimized clauses. diff --git a/packages/db/tests/query/optimizer-semantics-oracle.test.ts b/packages/db/tests/query/optimizer-semantics-oracle.test.ts index 48c6a85277..4247a7934c 100644 --- a/packages/db/tests/query/optimizer-semantics-oracle.test.ts +++ b/packages/db/tests/query/optimizer-semantics-oracle.test.ts @@ -23,14 +23,31 @@ import { describe, expect, test } from 'vitest' import { createCollection } from '../../src/collection/index.js' import { createLiveQueryCollection } from '../../src/query/index.js' +import { optimizeQuery } from '../../src/query/optimizer.js' import { add, caseWhen, eq, isUndefined, + or, sum, } from '../../src/query/builder/functions.js' +import { + CollectionRef, + Func, + PropRef, + QueryRef, + Value, + createResidualWhere, +} from '../../src/query/ir.js' import { mockSyncCollectionOptions, stripVirtualProps } from '../utils.js' +import type { CollectionImpl } from '../../src/collection/index.js' +import type { + BasicExpression, + From, + QueryIR, + Where, +} from '../../src/query/ir.js' type Row = { id: number; k: number; v: number } @@ -255,3 +272,519 @@ describe('optimizer aggregate semantics', () => { ]) }) }) + +/** + * A predicate may enter a nonnullable source once. An outer join still applies + * that predicate to the joined row, so its outer copy stays residual. A + * nullable-side predicate stays regular and must not enter its source. This + * follows the optimizer's stated predicate-pushdown and outer-join contract. + * + * Model: the table below states each join side's relational role. It does not + * use the optimizer's nullable-source classifier. `observedTerms` only reads + * the output IR; it retains duplicate predicates and residual markers. Plain + * array joins independently compute the expected first public snapshot. + * + * Bounded grammar: LEFT, RIGHT, INNER, and FULL joins; one or two pushable + * equality predicates; separate WHERE clauses or one AND; both clause orders; + * greater-than and nullable-field isUndefined controls; then a LEFT/INNER + * chain with two distinct pushable sources. Re-optimizing output checks the + * later-pass boundary. The public driver includes matched and unmatched rows + * and an OR predicate that accepts an unmatched row. + * + * The structural checkpoint is `optimizeQuery` return: each source and outer + * predicate has its expected exact multiplicity and marker. The public + * checkpoint is the first synchronous live-query Collection snapshot. These + * checks do not claim incremental histories, arbitrary functions, UNIONs, or + * a maximum wall-clock runtime. + */ +type ObservedTerm = { key: string; residual: boolean } + +function observedTerms(where: Array | undefined): Array { + const terms: Array = [] + for (const clause of where ?? []) { + const residual = `expression` in clause && clause.residual === true + const expression = `expression` in clause ? clause.expression : clause + const visit = (current: BasicExpression): void => { + if (current instanceof Func && current.name === `and`) { + current.args.forEach(visit) + return + } + if (current instanceof Func && current.name === `or`) { + const keys = current.args.map((arg) => { + if ( + !(arg instanceof Func) || + arg.name !== `eq` || + !(arg.args[0] instanceof PropRef) + ) { + throw new Error(`Unexpected OR term in residual oracle`) + } + return arg.args[0].path.join(`.`) + }) + terms.push({ key: `or(${keys.join(`,`)})`, residual }) + return + } + if ( + !(current instanceof Func) || + ![`eq`, `gt`, `isUndefined`].includes(current.name) || + !(current.args[0] instanceof PropRef) + ) { + throw new Error(`Unexpected optimizer predicate in residual oracle`) + } + const key = current.args[0].path.join(`.`) + const comparedRef = current.args[1] + terms.push({ + key: + current.name !== `eq` + ? `${current.name}(${key})` + : comparedRef instanceof PropRef + ? `${key}=${comparedRef.path.join(`.`)}` + : key, + residual, + }) + } + visit(expression) + } + return terms.sort( + (a, b) => + a.key.localeCompare(b.key) || Number(a.residual) - Number(b.residual), + ) +} + +function sourceTerms(source: From): Array { + if (source.type === `collectionRef`) return [] + if (source.type !== `queryRef`) { + throw new Error(`Unexpected optimizer source in residual oracle`) + } + return observedTerms(source.query.where) +} + +function predicate( + alias: string, + field: string, + value: unknown, +): Func { + return new Func(`eq`, [new PropRef([alias, field]), new Value(value)]) +} + +function inertCollection(id: string): CollectionImpl { + // The optimizer uses Collection identity but does not read Collection methods. + return { id } as unknown as CollectionImpl +} + +function expectedTerms( + keys: Array, + residual: boolean, +): Array { + return keys + .map((key) => ({ key, residual })) + .sort((a, b) => a.key.localeCompare(b.key)) +} + +describe(`optimizer residual convergence`, () => { + const joins = [ + { type: `left`, teamPush: true, memberPush: false, residual: true }, + { type: `right`, teamPush: false, memberPush: true, residual: true }, + { type: `inner`, teamPush: true, memberPush: true, residual: false }, + { type: `full`, teamPush: false, memberPush: false, residual: false }, + ] as const + + for (const join of joins) { + for (const form of [`separate`, `and`] as const) { + for (const reversed of [false, true]) { + for (const twoActive of [false, true]) { + test(`${join.type} ${form} reversed=${reversed} twoActive=${twoActive} preserves each predicate once`, () => { + const teamKeys = [`team.active`] + const memberKeys = [`member.userId`] + const clauses = [ + predicate(`team`, `active`, true), + predicate(`member`, `userId`, 100), + ] + if (twoActive) { + if (join.type === `right`) { + memberKeys.push(`member.role`) + clauses.push(predicate(`member`, `role`, `admin`)) + } else { + teamKeys.push(`team.region`) + clauses.push(predicate(`team`, `region`, `west`)) + } + } + const ordered = reversed ? [...clauses].reverse() : clauses + const where: Array = + form === `and` ? [new Func(`and`, ordered)] : ordered + const collection = inertCollection(`residual-oracle`) + const query: QueryIR = { + from: new CollectionRef(collection, `team`), + join: [ + { + type: join.type, + from: new CollectionRef(collection, `member`), + left: new PropRef([`team`, `id`]), + right: new PropRef([`member`, `teamId`]), + }, + ], + where, + } + + const optimized = optimizeQuery(query).optimizedQuery + const laterPass = optimizeQuery(optimized).optimizedQuery + for (const actual of [optimized, laterPass]) { + expect(sourceTerms(actual.from)).toEqual( + join.teamPush ? expectedTerms(teamKeys, false) : [], + ) + expect(sourceTerms(actual.join![0]!.from)).toEqual( + join.memberPush ? expectedTerms(memberKeys, false) : [], + ) + expect(observedTerms(actual.where)).toEqual( + [ + ...(!join.teamPush || join.residual + ? expectedTerms(teamKeys, join.teamPush) + : []), + ...(!join.memberPush || join.residual + ? expectedTerms(memberKeys, join.memberPush) + : []), + ].sort((a, b) => a.key.localeCompare(b.key)), + ) + expect(actual.where).toHaveLength( + join.type === `inner` ? 0 : join.type === `full` ? 1 : 2, + ) + } + }) + } + } + } + } + + test(`two active sources retain both residual predicates across passes`, () => { + const collection = inertCollection(`residual-multi-source`) + const query: QueryIR = { + from: new CollectionRef(collection, `team`), + join: [ + { + type: `left`, + from: new CollectionRef(collection, `member`), + left: new PropRef([`team`, `id`]), + right: new PropRef([`member`, `teamId`]), + }, + { + type: `inner`, + from: new CollectionRef(collection, `tag`), + left: new PropRef([`team`, `id`]), + right: new PropRef([`tag`, `teamId`]), + }, + ], + where: [ + predicate(`member`, `userId`, 100), + predicate(`team`, `active`, true), + predicate(`tag`, `region`, `west`), + ], + } + const optimized = optimizeQuery(query).optimizedQuery + for (const actual of [optimized, optimizeQuery(optimized).optimizedQuery]) { + expect(sourceTerms(actual.from)).toEqual( + expectedTerms([`team.active`], false), + ) + expect(sourceTerms(actual.join![0]!.from)).toEqual([]) + expect(sourceTerms(actual.join![1]!.from)).toEqual( + expectedTerms([`tag.region`], false), + ) + expect(observedTerms(actual.where)).toEqual([ + { key: `member.userId`, residual: false }, + { key: `tag.region`, residual: true }, + { key: `team.active`, residual: true }, + ]) + expect(actual.where).toHaveLength(2) + } + }) + + for (const joinType of [`left`, `right`] as const) { + test(`${joinType} keeps an OR clause residual and a cross-source clause regular`, () => { + const collection = inertCollection(`residual-clause-forms`) + const activeAlias = joinType === `left` ? `team` : `member` + const active = new Func(`or`, [ + predicate(activeAlias, `active`, true), + predicate(activeAlias, `region`, `west`), + ]) + const cross = new Func(`eq`, [ + new PropRef([`team`, `id`]), + new PropRef([`member`, `teamId`]), + ]) + const query: QueryIR = { + from: new CollectionRef(collection, `team`), + join: [ + { + type: joinType, + from: new CollectionRef(collection, `member`), + left: new PropRef([`team`, `id`]), + right: new PropRef([`member`, `teamId`]), + }, + ], + where: [cross, active], + } + const optimized = optimizeQuery(query).optimizedQuery + const activeKey = `or(${activeAlias}.active,${activeAlias}.region)` + const activeSource = + joinType === `left` ? optimized.from : optimized.join![0]!.from + expect(sourceTerms(activeSource)).toEqual( + expectedTerms([activeKey], false), + ) + expect(observedTerms(optimized.where)).toEqual( + [ + { key: activeKey, residual: true }, + { key: `team.id=member.teamId`, residual: false }, + ].sort((a, b) => a.key.localeCompare(b.key)), + ) + expect(optimized.where).toHaveLength(2) + }) + } + + test(`non-equality pushdown retains its residual while nullable checks remain regular`, () => { + const collection = inertCollection(`residual-predicate-kinds`) + const query: QueryIR = { + from: new CollectionRef(collection, `team`), + join: [ + { + type: `left`, + from: new CollectionRef(collection, `member`), + left: new PropRef([`team`, `id`]), + right: new PropRef([`member`, `teamId`]), + }, + ], + where: [ + new Func(`gt`, [new PropRef([`team`, `score`]), new Value(5)]), + new Func(`isUndefined`, [new PropRef([`member`, `optional`])]), + ], + } + + const optimized = optimizeQuery(query).optimizedQuery + for (const actual of [optimized, optimizeQuery(optimized).optimizedQuery]) { + expect(sourceTerms(actual.from)).toEqual( + expectedTerms([`gt(team.score)`], false), + ) + expect(sourceTerms(actual.join![0]!.from)).toEqual([]) + expect(observedTerms(actual.where)).toEqual([ + { key: `gt(team.score)`, residual: true }, + { key: `isUndefined(member.optional)`, residual: false }, + ]) + expect(actual.where).toHaveLength(2) + } + }) + + test(`a later pass keeps existing residuals while pushing a new predicate`, () => { + const collection = inertCollection(`residual-later-pass`) + const prior = predicate(`team`, `region`, `west`) + const newlyPushable = predicate(`team`, `active`, true) + const nullable = predicate(`member`, `userId`, 100) + const query: QueryIR = { + from: new QueryRef( + { from: new CollectionRef(collection, `team`), where: [prior] }, + `team`, + ), + join: [ + { + type: `left`, + from: new CollectionRef(collection, `member`), + left: new PropRef([`team`, `id`]), + right: new PropRef([`member`, `teamId`]), + }, + ], + where: [createResidualWhere(prior), nullable, newlyPushable], + } + const optimized = optimizeQuery(query).optimizedQuery + for (const actual of [optimized, optimizeQuery(optimized).optimizedQuery]) { + expect(sourceTerms(actual.from)).toEqual( + expectedTerms([`team.active`, `team.region`], false), + ) + expect(observedTerms(actual.where)).toEqual([ + { key: `member.userId`, residual: false }, + { key: `team.active`, residual: true }, + { key: `team.region`, residual: true }, + ]) + expect(actual.where).toHaveLength(2) + } + }) + + type Team = { id: number; active: boolean } + type Member = { id: number; teamId: number; userId: number } + const teams: Array = [ + { id: 1, active: true }, + { id: 2, active: false }, + { id: 3, active: true }, + ] + const members: Array = [ + { id: 10, teamId: 1, userId: 100 }, + { id: 11, teamId: 1, userId: 200 }, + { id: 12, teamId: 2, userId: 100 }, + { id: 13, teamId: 4, userId: 100 }, + ] + + for (const joinType of [`left`, `right`] as const) { + test(`${joinType} join publishes the independently joined rows`, () => { + const teamCollection = createCollection( + mockSyncCollectionOptions({ + id: `optimizer-residual-${joinType}-teams`, + getKey: (row) => row.id, + initialData: teams, + }), + ) + const memberCollection = createCollection( + mockSyncCollectionOptions({ + id: `optimizer-residual-${joinType}-members`, + getKey: (row) => row.id, + initialData: members, + }), + ) + + // The model fully joins arrays, including the unmatched preserved side, + // then applies the WHERE rule to each joined occurrence. + const pairs: Array<{ team?: Team; member?: Member }> = [] + for (const team of teams) { + const matching = members.filter((member) => member.teamId === team.id) + for (const member of matching) pairs.push({ team, member }) + if (matching.length === 0 && joinType === `left`) pairs.push({ team }) + } + if (joinType === `right`) { + for (const member of members) { + if (teams.every((team) => team.id !== member.teamId)) { + pairs.push({ member }) + } + } + } + const expected = pairs + .filter(({ team, member }) => + joinType === `left` + ? team?.active === true && + (member === undefined || member.userId === 100) + : member?.userId === 100 && + (team === undefined || team.active === true), + ) + .map(({ team, member }) => [team?.id ?? null, member?.id ?? null]) + .sort((a, b) => JSON.stringify(a).localeCompare(JSON.stringify(b))) + + const result = + joinType === `left` + ? createLiveQueryCollection({ + startSync: true, + query: (q) => + q + .from({ team: teamCollection }) + .leftJoin({ member: memberCollection }, ({ team, member }) => + eq(team.id, member.teamId), + ) + .where(({ team }) => eq(team.active, true)) + .where(({ member }) => + or(isUndefined(member.id), eq(member.userId, 100)), + ) + .select(({ team, member }) => ({ + teamId: team.id, + memberId: member.id, + })), + }) + : createLiveQueryCollection({ + startSync: true, + query: (q) => + q + .from({ team: teamCollection }) + .rightJoin({ member: memberCollection }, ({ team, member }) => + eq(team.id, member.teamId), + ) + .where(({ member }) => eq(member.userId, 100)) + .where(({ team }) => + or(isUndefined(team.id), eq(team.active, true)), + ) + .select(({ team, member }) => ({ + teamId: team.id, + memberId: member.id, + })), + }) + const actual = result.toArray + .map(stripVirtualProps) + .map((row) => { + if (row === undefined) throw new Error(`Missing joined result row`) + return [row.teamId ?? null, row.memberId ?? null] + }) + .sort((a, b) => JSON.stringify(a).localeCompare(JSON.stringify(b))) + expect(actual).toEqual(expected) + }) + } + + test(`two active sources publish the independently joined rows`, () => { + const tags = [ + { id: 20, teamId: 1, region: `west` }, + { id: 21, teamId: 2, region: `west` }, + { id: 22, teamId: 3, region: `west` }, + { id: 23, teamId: 3, region: `east` }, + ] + const teamCollection = createCollection( + mockSyncCollectionOptions({ + id: `optimizer-residual-chain-teams`, + getKey: (row) => row.id, + initialData: teams, + }), + ) + const memberCollection = createCollection( + mockSyncCollectionOptions({ + id: `optimizer-residual-chain-members`, + getKey: (row) => row.id, + initialData: members, + }), + ) + const tagCollection = createCollection( + mockSyncCollectionOptions({ + id: `optimizer-residual-chain-tags`, + getKey: (row: (typeof tags)[number]) => row.id, + initialData: tags, + }), + ) + + const expected = teams + .flatMap((team) => { + if (!team.active) return [] + const matchingMembers = members.filter( + (member) => member.teamId === team.id, + ) + const acceptedMembers = matchingMembers.length + ? matchingMembers.filter((member) => member.userId === 100) + : [undefined] + return tags + .filter((tag) => tag.teamId === team.id && tag.region === `west`) + .flatMap((tag) => + acceptedMembers.map((member) => [ + team.id, + member?.id ?? null, + tag.id, + ]), + ) + }) + .sort((a, b) => JSON.stringify(a).localeCompare(JSON.stringify(b))) + + const result = createLiveQueryCollection({ + startSync: true, + query: (q) => + q + .from({ team: teamCollection }) + .leftJoin({ member: memberCollection }, ({ team, member }) => + eq(team.id, member.teamId), + ) + .innerJoin({ tag: tagCollection }, ({ team, tag }) => + eq(team.id, tag.teamId), + ) + .where(({ member }) => + or(isUndefined(member.id), eq(member.userId, 100)), + ) + .where(({ team }) => eq(team.active, true)) + .where(({ tag }) => eq(tag.region, `west`)) + .select(({ team, member, tag }) => ({ + teamId: team.id, + memberId: member.id, + tagId: tag.id, + })), + }) + const actual = result.toArray + .map(stripVirtualProps) + .map((row) => { + return [row.teamId, row.memberId ?? null, row.tagId] + }) + .sort((a, b) => JSON.stringify(a).localeCompare(JSON.stringify(b))) + expect(actual).toEqual(expected) + }) +}) From 92490bc7245c734ad547746786ae9c0219fe81db Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 1 Oct 2026 12:34:07 -0600 Subject: [PATCH 3/4] fix(db): preserve source-free join filters --- packages/db/src/query/optimizer.ts | 5 +- .../query/optimizer-semantics-oracle.test.ts | 305 +++++++++++++++--- packages/db/tests/query/optimizer.test.ts | 75 ++--- 3 files changed, 292 insertions(+), 93 deletions(-) diff --git a/packages/db/src/query/optimizer.ts b/packages/db/src/query/optimizer.ts index 05fd8e78ab..0ca2babff1 100644 --- a/packages/db/src/query/optimizer.ts +++ b/packages/db/src/query/optimizer.ts @@ -678,11 +678,10 @@ function groupWhereClauses( singleSource.set(source, []) } singleSource.get(source)!.push(clause.expression) - } else if (clause.touchedSources.size > 1 || clause.hasNamespaceOnlyRef) { - // Multi-source clause or namespace-only reference - must stay in main query + } else { + // Clauses without one pushable source must stay in the main query. multiSource.push(clause.expression) } - // Skip clauses that touch no sources (constants) - they don't need optimization } // Combine multiple clauses for each source with AND diff --git a/packages/db/tests/query/optimizer-semantics-oracle.test.ts b/packages/db/tests/query/optimizer-semantics-oracle.test.ts index 4247a7934c..11c0d36ae7 100644 --- a/packages/db/tests/query/optimizer-semantics-oracle.test.ts +++ b/packages/db/tests/query/optimizer-semantics-oracle.test.ts @@ -286,13 +286,17 @@ describe('optimizer aggregate semantics', () => { * * Bounded grammar: LEFT, RIGHT, INNER, and FULL joins; one or two pushable * equality predicates; separate WHERE clauses or one AND; both clause orders; - * greater-than and nullable-field isUndefined controls; then a LEFT/INNER - * chain with two distinct pushable sources. Re-optimizing output checks the - * later-pass boundary. The public driver includes matched and unmatched rows - * and an OR predicate that accepts an unmatched row. + * greater-than, nullable-field and namespace-only isUndefined controls; then + * a LEFT/INNER chain with two distinct pushable sources and chains ending in + * RIGHT/FULL. A source-free false clause and an ordered QueryRef that declines + * pushdown check preservation of regular predicates. Re-optimizing output + * checks the later-pass boundary. The public driver includes matched and + * unmatched rows, an OR predicate that accepts an unmatched row, and a + * source-free false predicate that rejects every row. * * The structural checkpoint is `optimizeQuery` return: each source and outer - * predicate has its expected exact multiplicity and marker. The public + * predicate has its expected exact multiplicity and marker, and the extracted + * source clauses exclude nullable sources. The public * checkpoint is the first synchronous live-query Collection snapshot. These * checks do not claim incremental histories, arbitrary functions, UNIONs, or * a maximum wall-clock runtime. @@ -314,11 +318,12 @@ function observedTerms(where: Array | undefined): Array { if ( !(arg instanceof Func) || arg.name !== `eq` || - !(arg.args[0] instanceof PropRef) + !(arg.args[0] instanceof PropRef) || + !(arg.args[1] instanceof Value) ) { throw new Error(`Unexpected OR term in residual oracle`) } - return arg.args[0].path.join(`.`) + return `${arg.args[0].path.join(`.`)}=${JSON.stringify(arg.args[1].value)}` }) terms.push({ key: `or(${keys.join(`,`)})`, residual }) return @@ -330,17 +335,22 @@ function observedTerms(where: Array | undefined): Array { ) { throw new Error(`Unexpected optimizer predicate in residual oracle`) } - const key = current.args[0].path.join(`.`) - const comparedRef = current.args[1] - terms.push({ - key: - current.name !== `eq` - ? `${current.name}(${key})` - : comparedRef instanceof PropRef - ? `${key}=${comparedRef.path.join(`.`)}` - : key, - residual, - }) + const path = current.args[0].path.join(`.`) + const compared = current.args[1] + let key: string + if (current.name === `isUndefined`) { + key = `isUndefined(${path})` + } else if (compared instanceof Value) { + key = + current.name === `gt` + ? `gt(${path},${JSON.stringify(compared.value)})` + : `${path}=${JSON.stringify(compared.value)}` + } else if (current.name === `eq` && compared instanceof PropRef) { + key = `${path}=${compared.path.join(`.`)}` + } else { + throw new Error(`Unexpected compared value in residual oracle`) + } + terms.push({ key, residual }) } visit(expression) } @@ -381,6 +391,21 @@ function expectedTerms( } describe(`optimizer residual convergence`, () => { + test(`structural recorder distinguishes changed predicate values`, () => { + expect(observedTerms([predicate(`team`, `active`, true)])).not.toEqual( + observedTerms([predicate(`team`, `active`, false)]), + ) + expect( + observedTerms([ + new Func(`gt`, [new PropRef([`team`, `score`]), new Value(5)]), + ]), + ).not.toEqual( + observedTerms([ + new Func(`gt`, [new PropRef([`team`, `score`]), new Value(6)]), + ]), + ) + }) + const joins = [ { type: `left`, teamPush: true, memberPush: false, residual: true }, { type: `right`, teamPush: false, memberPush: true, residual: true }, @@ -393,18 +418,18 @@ describe(`optimizer residual convergence`, () => { for (const reversed of [false, true]) { for (const twoActive of [false, true]) { test(`${join.type} ${form} reversed=${reversed} twoActive=${twoActive} preserves each predicate once`, () => { - const teamKeys = [`team.active`] - const memberKeys = [`member.userId`] + const teamKeys = [`team.active=true`] + const memberKeys = [`member.userId=100`] const clauses = [ predicate(`team`, `active`, true), predicate(`member`, `userId`, 100), ] if (twoActive) { if (join.type === `right`) { - memberKeys.push(`member.role`) + memberKeys.push(`member.role="admin"`) clauses.push(predicate(`member`, `role`, `admin`)) } else { - teamKeys.push(`team.region`) + teamKeys.push(`team.region="west"`) clauses.push(predicate(`team`, `region`, `west`)) } } @@ -425,7 +450,25 @@ describe(`optimizer residual convergence`, () => { where, } - const optimized = optimizeQuery(query).optimizedQuery + const result = optimizeQuery(query) + const optimized = result.optimizedQuery + expect([...result.sourceWhereClauses.keys()].sort()).toEqual( + [ + ...(join.teamPush ? [`team`] : []), + ...(join.memberPush ? [`member`] : []), + ].sort(), + ) + for (const [alias, keys] of [ + [`team`, teamKeys], + [`member`, memberKeys], + ] as const) { + const clause = result.sourceWhereClauses.get(alias) + if (clause) { + expect(observedTerms([clause])).toEqual( + expectedTerms(keys, false), + ) + } + } const laterPass = optimizeQuery(optimized).optimizedQuery for (const actual of [optimized, laterPass]) { expect(sourceTerms(actual.from)).toEqual( @@ -481,18 +524,17 @@ describe(`optimizer residual convergence`, () => { const optimized = optimizeQuery(query).optimizedQuery for (const actual of [optimized, optimizeQuery(optimized).optimizedQuery]) { expect(sourceTerms(actual.from)).toEqual( - expectedTerms([`team.active`], false), + expectedTerms([`team.active=true`], false), ) expect(sourceTerms(actual.join![0]!.from)).toEqual([]) expect(sourceTerms(actual.join![1]!.from)).toEqual( - expectedTerms([`tag.region`], false), + expectedTerms([`tag.region="west"`], false), ) expect(observedTerms(actual.where)).toEqual([ - { key: `member.userId`, residual: false }, - { key: `tag.region`, residual: true }, - { key: `team.active`, residual: true }, + { key: `member.userId=100`, residual: false }, + { key: `tag.region="west"`, residual: true }, + { key: `team.active=true`, residual: true }, ]) - expect(actual.where).toHaveLength(2) } }) @@ -521,7 +563,7 @@ describe(`optimizer residual convergence`, () => { where: [cross, active], } const optimized = optimizeQuery(query).optimizedQuery - const activeKey = `or(${activeAlias}.active,${activeAlias}.region)` + const activeKey = `or(${activeAlias}.active=true,${activeAlias}.region="west")` const activeSource = joinType === `left` ? optimized.from : optimized.join![0]!.from expect(sourceTerms(activeSource)).toEqual( @@ -533,7 +575,6 @@ describe(`optimizer residual convergence`, () => { { key: `team.id=member.teamId`, residual: false }, ].sort((a, b) => a.key.localeCompare(b.key)), ) - expect(optimized.where).toHaveLength(2) }) } @@ -558,14 +599,172 @@ describe(`optimizer residual convergence`, () => { const optimized = optimizeQuery(query).optimizedQuery for (const actual of [optimized, optimizeQuery(optimized).optimizedQuery]) { expect(sourceTerms(actual.from)).toEqual( - expectedTerms([`gt(team.score)`], false), + expectedTerms([`gt(team.score,5)`], false), ) expect(sourceTerms(actual.join![0]!.from)).toEqual([]) expect(observedTerms(actual.where)).toEqual([ - { key: `gt(team.score)`, residual: true }, + { key: `gt(team.score,5)`, residual: true }, { key: `isUndefined(member.optional)`, residual: false }, ]) - expect(actual.where).toHaveLength(2) + } + }) + + test(`a source-free false predicate survives optimization passes`, () => { + const collection = inertCollection(`residual-constant`) + const query: QueryIR = { + from: new CollectionRef(collection, `team`), + join: [ + { + type: `left`, + from: new CollectionRef(collection, `member`), + left: new PropRef([`team`, `id`]), + right: new PropRef([`member`, `teamId`]), + }, + ], + where: [predicate(`team`, `active`, true), new Value(false)], + } + + for (const actual of [ + optimizeQuery(query).optimizedQuery, + optimizeQuery(optimizeQuery(query).optimizedQuery).optimizedQuery, + ]) { + expect( + actual.where?.some( + (clause) => clause instanceof Value && clause.value === false, + ), + ).toBe(true) + } + }) + + test(`a namespace predicate stays outside a nullable source`, () => { + const collection = inertCollection(`residual-namespace`) + const query: QueryIR = { + from: new CollectionRef(collection, `team`), + join: [ + { + type: `left`, + from: new CollectionRef(collection, `member`), + left: new PropRef([`team`, `id`]), + right: new PropRef([`member`, `teamId`]), + }, + ], + where: [ + predicate(`team`, `active`, true), + new Func(`isUndefined`, [new PropRef([`member`])]), + ], + } + + const result = optimizeQuery(query) + expect([...result.sourceWhereClauses.keys()]).toEqual([`team`]) + for (const actual of [ + result.optimizedQuery, + optimizeQuery(result.optimizedQuery).optimizedQuery, + ]) { + expect(sourceTerms(actual.from)).toEqual( + expectedTerms([`team.active=true`], false), + ) + expect(sourceTerms(actual.join![0]!.from)).toEqual([]) + expect(observedTerms(actual.where)).toEqual([ + { key: `isUndefined(member)`, residual: false }, + { key: `team.active=true`, residual: true }, + ]) + } + }) + + for (const laterJoin of [`right`, `full`] as const) { + test(`a later ${laterJoin} join keeps earlier aliases nullable`, () => { + const collection = inertCollection(`residual-later-${laterJoin}`) + const query: QueryIR = { + from: new CollectionRef(collection, `team`), + join: [ + { + type: `left`, + from: new CollectionRef(collection, `member`), + left: new PropRef([`team`, `id`]), + right: new PropRef([`member`, `teamId`]), + }, + { + type: laterJoin, + from: new CollectionRef(collection, `tag`), + left: new PropRef([`team`, `id`]), + right: new PropRef([`tag`, `teamId`]), + }, + ], + where: [ + predicate(`team`, `active`, true), + predicate(`member`, `userId`, 100), + predicate(`tag`, `region`, `west`), + ], + } + + const result = optimizeQuery(query) + expect([...result.sourceWhereClauses.keys()]).toEqual( + laterJoin === `right` ? [`tag`] : [], + ) + for (const actual of [ + result.optimizedQuery, + optimizeQuery(result.optimizedQuery).optimizedQuery, + ]) { + expect(sourceTerms(actual.from)).toEqual([]) + expect(sourceTerms(actual.join![0]!.from)).toEqual([]) + expect(sourceTerms(actual.join![1]!.from)).toEqual( + laterJoin === `right` + ? expectedTerms([`tag.region="west"`], false) + : [], + ) + expect(observedTerms(actual.where)).toEqual([ + { key: `member.userId=100`, residual: false }, + { key: `tag.region="west"`, residual: laterJoin === `right` }, + { key: `team.active=true`, residual: false }, + ]) + } + }) + } + + test(`an ordered QueryRef that declines pushdown keeps its outer predicate`, () => { + const collection = inertCollection(`residual-declined-queryref`) + const query: QueryIR = { + from: new QueryRef( + { + from: new CollectionRef(collection, `team`), + orderBy: [ + { + expression: new PropRef([`team`, `id`]), + compareOptions: { + direction: `desc`, + nulls: `first`, + stringSort: `locale`, + }, + }, + ], + limit: 1, + }, + `team`, + ), + join: [ + { + type: `left`, + from: new CollectionRef(collection, `member`), + left: new PropRef([`team`, `id`]), + right: new PropRef([`member`, `teamId`]), + }, + ], + where: [ + predicate(`team`, `active`, true), + predicate(`member`, `userId`, 100), + ], + } + + const result = optimizeQuery(query) + expect(result.sourceWhereClauses.size).toBe(0) + for (const actual of [ + result.optimizedQuery, + optimizeQuery(result.optimizedQuery).optimizedQuery, + ]) { + expect(sourceTerms(actual.from)).toEqual([]) + expect(observedTerms(actual.where)).toEqual( + expectedTerms([`member.userId=100`, `team.active=true`], false), + ) } }) @@ -592,12 +791,12 @@ describe(`optimizer residual convergence`, () => { const optimized = optimizeQuery(query).optimizedQuery for (const actual of [optimized, optimizeQuery(optimized).optimizedQuery]) { expect(sourceTerms(actual.from)).toEqual( - expectedTerms([`team.active`, `team.region`], false), + expectedTerms([`team.active=true`, `team.region="west"`], false), ) expect(observedTerms(actual.where)).toEqual([ - { key: `member.userId`, residual: false }, - { key: `team.active`, residual: true }, - { key: `team.region`, residual: true }, + { key: `member.userId=100`, residual: false }, + { key: `team.active=true`, residual: true }, + { key: `team.region="west"`, residual: true }, ]) expect(actual.where).toHaveLength(2) } @@ -617,6 +816,36 @@ describe(`optimizer residual convergence`, () => { { id: 13, teamId: 4, userId: 100 }, ] + test(`a source-free false predicate publishes no joined rows`, () => { + const teamCollection = createCollection( + mockSyncCollectionOptions({ + id: `optimizer-residual-constant-team`, + getKey: (row) => row.id, + initialData: teams, + }), + ) + const memberCollection = createCollection( + mockSyncCollectionOptions({ + id: `optimizer-residual-constant-member`, + getKey: (row) => row.id, + initialData: members, + }), + ) + const result = createLiveQueryCollection({ + startSync: true, + query: (q) => + q + .from({ team: teamCollection }) + .leftJoin({ member: memberCollection }, ({ team, member }) => + eq(team.id, member.teamId), + ) + .where(({ team }) => eq(team.active, true)) + .where(() => new Value(false)), + }) + + expect(result.toArray).toEqual([]) + }) + for (const joinType of [`left`, `right`] as const) { test(`${joinType} join publishes the independently joined rows`, () => { const teamCollection = createCollection( diff --git a/packages/db/tests/query/optimizer.test.ts b/packages/db/tests/query/optimizer.test.ts index babeabca93..12cbf20982 100644 --- a/packages/db/tests/query/optimizer.test.ts +++ b/packages/db/tests/query/optimizer.test.ts @@ -7,7 +7,6 @@ import { PropRef, QueryRef, Value, - createResidualWhere, } from '../../src/query/ir.js' import type { QueryIR } from '../../src/query/ir.js' @@ -492,8 +491,10 @@ describe(`Query Optimizer`, () => { const { optimizedQuery: optimized } = optimizeQuery(query) - // The constant expression should be ignored, single-source clause should be optimized - expect(optimized.where).toEqual([]) + // Source-free clauses still filter the joined result. + expect(optimized.where).toEqual([ + createEq(createValue(1), createValue(1)), + ]) expect(optimized.from.type).toBe(`queryRef`) if (optimized.from.type === `queryRef`) { expect(optimized.from.query.where).toHaveLength(1) @@ -662,8 +663,10 @@ describe(`Query Optimizer`, () => { const { optimizedQuery: optimized } = optimizeQuery(query) - // The empty path PropRef should be treated as a constant (no sources) - expect(optimized.where).toEqual([]) + // An unqualified reference cannot be pushed to either source. + expect(optimized.where).toEqual([ + createEq(emptyPathPropRef, createValue(1)), + ]) expect(optimized.from.type).toBe(`collectionRef`) }) @@ -688,10 +691,13 @@ describe(`Query Optimizer`, () => { const { optimizedQuery: optimized } = optimizeQuery(query) - // Multi-source clause should remain in main query + // Multi-source and source-free clauses should remain in the main query. expect(optimized.where).toHaveLength(1) expect(optimized.where![0]).toEqual( - createEq(createPropRef(`u`, `id`), createPropRef(`p`, `user_id`)), + createAnd( + createEq(createPropRef(`u`, `id`), createPropRef(`p`, `user_id`)), + createEq(createValue(1), createValue(1)), + ), ) // Single-source clauses should be moved to subqueries @@ -712,7 +718,7 @@ describe(`Query Optimizer`, () => { }) describe(`Error Handling`, () => { - test(`should handle malformed expressions gracefully`, () => { + test(`does not silently discard a malformed expression`, () => { const malformedExpression = { type: `unknown`, value: `test`, @@ -733,9 +739,8 @@ describe(`Query Optimizer`, () => { const { optimizedQuery: optimized } = optimizeQuery(query) - // Should not crash and should handle the malformed expression gracefully expect(optimized).toBeDefined() - expect(optimized.where).toEqual([]) + expect(optimized.where).toEqual([malformedExpression]) }) test(`should handle PropRef with empty first element`, () => { @@ -758,8 +763,10 @@ describe(`Query Optimizer`, () => { const { optimizedQuery: optimized } = optimizeQuery(query) - // PropRef with empty first element should be ignored, other clause should be optimized - expect(optimized.where).toEqual([]) + // Keep the unqualified clause instead of silently removing a filter. + expect(optimized.where).toEqual([ + createEq(propRefWithEmptyFirst, createValue(1)), + ]) expect(optimized.from.type).toBe(`queryRef`) if (optimized.from.type === `queryRef`) { expect(optimized.from.query.where).toHaveLength(1) @@ -789,8 +796,10 @@ describe(`Query Optimizer`, () => { const { optimizedQuery: optimized } = optimizeQuery(query) - // PropRef with undefined first element should be ignored, other clause should be optimized - expect(optimized.where).toEqual([]) + // Keep the unqualified clause instead of silently removing a filter. + expect(optimized.where).toEqual([ + createEq(propRefWithUndefinedFirst, createValue(1)), + ]) expect(optimized.from.type).toBe(`queryRef`) if (optimized.from.type === `queryRef`) { expect(optimized.from.query.where).toHaveLength(1) @@ -1698,44 +1707,6 @@ describe(`Query Optimizer`, () => { expect(optimizedQuery.join![0]!.from.type).toBe(`collectionRef`) }) - test(`should push a LEFT JOIN active-side clause down once when a nullable-side clause remains`, () => { - const teamsCollection = { id: `teams` } as any - const teamMembersCollection = { id: `team-members` } as any - - const memberFilter = createEq( - createPropRef(`teamMember`, `user_id`), - createValue(100), - ) - const teamFilter = createEq( - createPropRef(`team`, `active`), - createValue(true), - ) - - const query: QueryIR = { - from: new CollectionRef(teamsCollection, `team`), - join: [ - { - type: `left`, - from: new CollectionRef(teamMembersCollection, `teamMember`), - left: createPropRef(`team`, `id`), - right: createPropRef(`teamMember`, `team_id`), - }, - ], - where: [memberFilter, teamFilter], - } - - const { optimizedQuery } = optimizeQuery(query) - - expect(optimizedQuery.from.type).toBe(`queryRef`) - expect((optimizedQuery.from as QueryRef).query.where).toEqual([ - teamFilter, - ]) - expect(optimizedQuery.where).toEqual([ - memberFilter, - createResidualWhere(teamFilter), - ]) - }) - test(`should preserve WHERE clause semantics when pushing down to RIGHT JOIN`, () => { // This test reproduces the bug where pushing WHERE clauses into RIGHT JOIN subqueries // changes the semantics by filtering out null values that should remain From 31502a232cba5f8d87a9b60759600eada72aa0bb Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 1 Oct 2026 12:35:26 -0600 Subject: [PATCH 4/4] docs: record optimizer loss-audit evidence --- .changeset/optimizer-residual-convergence.md | 2 +- docs/contributing/oracle-coverage.md | 11 ++++ ...2026-10-01-pr-1977-residual-convergence.md | 55 +++++++++++++++++-- 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/.changeset/optimizer-residual-convergence.md b/.changeset/optimizer-residual-convergence.md index 981e03e907..05563cfb82 100644 --- a/.changeset/optimizer-residual-convergence.md +++ b/.changeset/optimizer-residual-convergence.md @@ -2,4 +2,4 @@ '@tanstack/db': patch --- -Keep residual WHERE clauses marked as residual when the optimizer combines the remaining clauses of an outer-join query, so predicates pushed into a subquery are not pushed again on every pass until the iteration limit. +Keep pushed outer-join predicates residual across optimizer passes and preserve source-free WHERE clauses on joins. This prevents repeated filters and ensures joined queries still honor constant conditions. diff --git a/docs/contributing/oracle-coverage.md b/docs/contributing/oracle-coverage.md index 9c005acb33..af910bf784 100644 --- a/docs/contributing/oracle-coverage.md +++ b/docs/contributing/oracle-coverage.md @@ -594,6 +594,17 @@ boundary. Track the generalized repairs here instead of accumulating isolated regressions. Completion requires an executable owner, a production-path witness, a hostile wrong-answer control, and an explicit statement of remaining limits. +- [ ] **Optimizer residual convergence beyond the checked paths.** The + optimizer-semantics owner now checks `sourceWhereClauses`, source-free + predicates, namespace-only predicates, later RIGHT/FULL join nullability, + and an ordered QueryRef refusal at structural checkpoints. It still needs + public-row witnesses for later RIGHT/FULL chains, residual checks for + aggregate, functional, and projection-remapped QueryRef refusals, a + separate UNION branch model, and incremental source/publication histories. + Owner: `packages/db/tests/query/optimizer-semantics-oracle.test.ts` and + its production compiler driver. Each extension needs a distinguishing + wrong-plan control at the relevant checkpoint. + - [x] **Real-provider conformance fixtures.** Frozen 15.2.7 React Native and Node receipts cover the supported peer version; 18.2.1 React Native, Node, and browser receipts cover the known forward shapes. Exact-row checks and a diff --git a/docs/contributing/oracle-reviews/2026-10-01-pr-1977-residual-convergence.md b/docs/contributing/oracle-reviews/2026-10-01-pr-1977-residual-convergence.md index 7dd7f5a580..1879a08b29 100644 --- a/docs/contributing/oracle-reviews/2026-10-01-pr-1977-residual-convergence.md +++ b/docs/contributing/oracle-reviews/2026-10-01-pr-1977-residual-convergence.md @@ -10,6 +10,11 @@ and oracle file has SHA-256 `bd5b06605814d3f1c190455438974f9112a53219930ec7d8b92673917d3dc56c`. The separate task-local external-feedback ledger preserves all 13 PR and bot items; this record preserves the oracle and repair evidence. +The loss-audit follow-up reviewed PR head +`0da9cb9a0612e0b905723dffd2791c27978c6ec2` against the guide and +production paths. The code-bearing follow-up head is +`92490bc7245c734ad547746786ae9c0219fe81db`. This review record is an +evidence-only successor to that head. ## Contract and checked boundary @@ -43,9 +48,9 @@ live-query Collection, with matched and unmatched source rows. | Candidate, optimizer, join, and join-subquery suites | 247 passed after the predicate-kind case was added. | | Wrong-design controls | Marking regular clauses residual fails on `member.userId`'s marker. Keeping only the first residual fails on the missing `tag.region` predicate. Both are assertion failures at the structural checkpoint, not setup failures or timeouts. | -The production change adds 20 lines and removes 28, net **minus eight**. -The expanded executable oracle adds 533 lines. This separates the cost of the -repair from the broader test and documentation coverage. +At the initial reviewed candidate, the production change added 20 lines and +removed 28, net **minus eight**. The executable oracle added 533 lines. The +loss-audit follow-up has separate code-weight figures below. ## Oracle guide audit @@ -54,7 +59,7 @@ repair from the broader test and documentation coverage. | ORC-001 | The contract above states the optimizer and outer-join law, authority, finite grammar, and observations. The coverage map owns omitted expressions, join chains, and publication histories. | | ORC-002 | The structural expectation is an explicit relational-side table, not the optimizer's nullable-source classifier. The public result models join and filter plain arrays; they import neither optimizer analysis nor compiler evaluation. | | ORC-003 | The oracle opening states the contract, model, grammar, driver, and checkpoints. The table and array models give the answer; `optimizeQuery` and `createLiveQueryCollection` are the production drivers; assertions are adjacent to each case. | -| ORC-004 | Not applicable: this is a finite, enumerated grammar, not a generated-history property. Its named cross product and fixed extensions are the claim's bound. | +| ORC-004 | The finite matrix claims only its stated input grammar. Reconstruction: each of 4 join kinds × 2 clause forms × 2 clause orders × 2 predicate counts is constructed and run. Ablation: join kind changes nullable-side eligibility; clause form tests residual-marker retention after combination; order checks order independence; the extra predicate checks same-source multiplicity. Range: one or two active predicates, with the FULL no-push case, are the bounds. Exclusion: unknown source aliases are outside this valid-query grammar; UNION, arbitrary functions, and incremental histories are outside the claim. This records the controls even though the guide distinguishes finite enumeration from an important generated property. | | ORC-005 | Structural work is observed at the `optimizeQuery` return and after a later pass. The public driver observes exact projected rows at the first synchronous Collection snapshot. The recorder preserves duplicate predicate terms and result rows. | | ORC-006 | The original implementation and two wrong designs fail at the intended assertions. The contributor's partial fix also fails the later-pass clause-count assertion. | | ORC-007 | Not applicable: there is no important generated property or random campaign. Every enumerated case runs in the package oracle test file. | @@ -80,3 +85,45 @@ successfully. The full `packages/db` Vitest suite then passed 229 files and 7,937 tests with no type errors using `--testTimeout=120000`. This resolves the earlier self-package declaration setup errors. The structural and public history bounds above remain unchanged. + +## Loss-audit follow-up + +The audit found a real adjacent predicate-preservation defect: with a join, +`groupWhereClauses` silently discarded a clause that touched no source. A +`Value(false)` after an active-side filter therefore vanished, and the first +public live-query Collection snapshot contained three rows instead of none. +The structural predicate-preservation assertion and the public snapshot test +both failed before the repair and passed afterward. The repair retains every +clause without exactly one pushable source in the outer WHERE. Three older +optimizer tests expected source-free or malformed clauses to disappear; their +false-green expectations now assert preservation. + +The structural matrix now checks both `optimizedQuery` and +`sourceWhereClauses`, including nullable-source exclusion. Replacing the latter +with an empty map failed the left-join matrix case at the intended assertion +(`[]` versus `['team']`). Fixed extensions check a namespace-only predicate, +later RIGHT/FULL joins that make earlier aliases nullable, and an ordered +QueryRef that declines pushdown. Each checks the first and later optimizer +pass. The source-free case also checks public rows. These extensions do not +establish public results for the later join chains or all QueryRef refusal +reasons. UNION remains a separate optimizer path. + +The parallel preparation review found that the structural term reader ignored +predicate values. It could not distinguish `team.active=true` from +`team.active=false`. The reader now records literal values and rejects +unexpected compared expressions. A calibration test failed when an equality +literal was deliberately hidden, then passed with the corrected reader. The +main matrix also checks those values in pushed and residual predicates. +The duplicate focused LEFT JOIN test was removed because the matrix receives +its trace and its original-implementation RED evidence. + +The final branch diff against the fetched `origin/main` adds 22 production +lines and removes 31, net **minus nine**. The executable oracle adds 762 lines; +the existing optimizer tests add 23 and remove 13. Tests and review documents +are reported separately from production code. + +At the code-bearing head, the full `packages/db/tests` suite passed 197 files +and 7,675 tests with two Vitest threads and a 120-second test timeout. The +package TypeScript check, focused ESLint, Prettier, and diff check passed. +An earlier sandboxed full run could not write Vitest's temporary cache and +failed its replay calibration; the same suite passed with that cache writable.