Skip to content

fix(db): keep residual clauses residual when combining WHERE clauses - #1977

Open
Jaime02 wants to merge 1 commit into
TanStack:mainfrom
Jaime02:fix/optimizer-residual-convergence
Open

Jaime02 wants to merge 1 commit into
TanStack:mainfrom
Jaime02:fix/optimizer-residual-convergence

Conversation

@Jaime02

@Jaime02 Jaime02 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

🎯 Changes

Follow-up to #1976, found while debugging it. When a query has an outer join and two or more WHERE clauses remain, optimizeQuery never converges: it pushes the same predicate into the subquery on every pass until maxIterations (10).

For a left join with one nullable-side predicate and one active-side predicate:

from(part).leftJoin(ref, …).where(eq(ref.clientId, 1)).where(eq(part.active, true))

the optimized part subquery ended up as:

where: [ and(active, active, active, active, active, active, active, active, active), active ]

Cause

In applyOptimizations, a predicate pushed into an outer-join source is kept as a residual clause (createResidualWhere). applySingleLevelOptimization skips residual clauses so they aren't pushed again. When more than one clause remains, though, they're merged with:

combineWithAnd(remainingWhereClauses.flatMap((c) => splitAndClausesRecursive(getWhereExpression(c))))

getWhereExpression unwraps the residual marker, so the merged and(...) is a plain clause. On the next pass it's split again and the active-side predicate is pushed down again. This repeats every iteration.

The query result is still correct (the duplicated conjuncts are idempotent), but it costs up to 10 optimizer passes and leaves redundant filter work in the compiled pipeline.

Fix

Regular and residual clauses are combined separately. The residual group stays wrapped in createResidualWhere, so the next pass leaves it alone and the loop converges after one push.

Tests

New case in tests/query/optimizer.test.ts (JOIN semantics preservation): a left join with one nullable-side and one active-side predicate. The test asserts that the active-side predicate is pushed into the subquery exactly once and stays residual in the outer query. It fails on main and passes with this change.

✅ Checklist

  • I have tested this code locally with pnpm test. I ran the full packages/db vitest suite. Two property-test files hit the 5 s timeout on my machine, on main too. With --testTimeout=120000 the only failure is collection-subscription-lifecycle-publication with random seed -533529595, which fails the same way on main.

🚀 Release Impact

  • This change affects published code, and I have generated a changeset.
  • This change is docs/CI/dev-only (no release).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed query optimization for filters used with LEFT JOINs. Filters are now kept in the appropriate part of the query as optimizations are applied, preventing repeated processing and preserving correct results. This improves handling of queries with conditions on both sides of a join.

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 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 52502135-d969-4a16-852f-ac92969868df

📥 Commits

Reviewing files that changed from the base of the PR and between 18abcee and e6a1e29.

📒 Files selected for processing (3)
  • .changeset/optimizer-residual-convergence.md
  • packages/db/src/query/optimizer.ts
  • packages/db/tests/query/optimizer.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The optimizer now combines regular and residual WHERE clauses separately. It preserves residual markers when combining multiple residual clauses. A LEFT JOIN test checks which filters remain in the main query and which filter is pushed into a source subquery.

Changes

Residual WHERE handling

Layer / File(s) Summary
Preserve residual markers during clause combination
packages/db/src/query/optimizer.ts, packages/db/tests/query/optimizer.test.ts, .changeset/optimizer-residual-convergence.md
The optimizer combines regular clauses separately from residual clauses and re-marks combined residual clauses. The LEFT JOIN test checks that the team filter is pushed into the active-side source subquery, while the member filter and residual team filter remain in the main query. The changeset records the patch.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: kyleamathews

Merge Risk: ⚪ Minimal · up to e6a1e

The change preserves residual predicates to prevent repeated pushdown during optimization. No actionable merge-blocking risk is identified; merge after normal checks pass.

Architecture Summary

Architecture risk: 🔵 Low · up to e6a1e

The change affects 1 system.

Changed systems: packages/db

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — packages/db (library) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in packages/db/src/query/optimizer.ts: applyOptimizations now combines non-residual clauses separately from residual clauses. Multiple residual clauses are combined and re-marked as residual; previously, all remaining clauses were combined together, which could discard residual markers.
  • observed — Modified behavior in packages/db/tests/query/optimizer.test.ts: The test file now imports createResidualWhere from the query IR module.
  • observed — Modified behavior in packages/db/tests/query/optimizer.test.ts: Adds a test asserting that with a LEFT JOIN and both member and team filters, the team filter is pushed into the active-side source subquery, while the member filter and createResidualWhere(teamFilter) remain in the main query.
  • observed — Modified behavior in .changeset/optimizer-residual-convergence.md: Adds a patch changeset for @tanstack/db documenting residual-clause handling for optimizer passes on outer-join queries.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preserving residual clauses when combining WHERE clauses in the database optimizer.
Description check ✅ Passed The description is complete and follows the repository template. It explains the problem, cause, fix, tests, test limitations, checklist status, and release impact, and it documents the generated chan…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant