Skip to content

fix(db): preserve source identity in optimizer copies - #1976

Open
Jaime02 wants to merge 1 commit into
TanStack:mainfrom
Jaime02:fix/optimizer-preserve-source-ids
Open

Jaime02 wants to merge 1 commit into
TanStack:mainfrom
Jaime02:fix/optimizer-preserve-source-ids

Conversation

@Jaime02

@Jaime02 Jaime02 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

🎯 Changes

Fixes a regression in 0.11.0: a live query returns no rows when a from() subquery joins a collection under an alias that an included (materialize() / includes) subquery reuses.

q.from({
  part: q
    .from({ part: parts })
    .leftJoin({ ref: refs }, ({ part, ref }) => eq(ref.partId, part.id))
    .where(({ ref }) => eq(ref.clientId, 1))
    .where(({ part }) => eq(part.active, true))
    .select(({ part }) => part),
}).select(({ part }) => ({
  ...part,
  // same alias `ref` as the join above
  refs: materialize(q.from({ ref: refs }).where(({ ref }) => eq(ref.partId, part.id))),
}))
// 0.9.2 → [part 1]   0.11.0 → []

Cause

git bisect between @tanstack/db@0.9.2 and @tanstack/db@0.11.0 points to #1877 (40a5aea5, "preserve pushed subquery predicates"). That change is correct, but it exposed an older problem in the optimizer:

  1. The optimizer copies CollectionRefs with new CollectionRefClass(collection, alias). Every copy gets a new sourceId (deepCopyFrom, both branches of optimizeFromWithTracking, removeRedundantFromClause).
  2. Before fix(db): preserve pushed subquery predicates #1877, processFrom and processJoinSource compiled the user's original subquery IR, so these copies were never compiled. Since fix(db): preserve pushed subquery predicates #1877 they compile the optimized IR whenever it differs from the original.
  3. Inputs are keyed by sourceId, so a copy's ID matches no input. It falls back to allInputs[alias]. bindSourceInputs registers every raw source under its alias too, and the last one wins. collectCollectionSources visits the included subquery after the join, so allInputs["ref"] is the included subquery's input.
  4. The join therefore reads the includes child's stream, which only loads rows for parents that already exist. Lazy join demand goes to the original source (resolveLazySource prefers the lexical source), and nothing reads that source anymore. The join never matches, so no parent rows are produced.

The optimized IR differs only when a predicate is pushed down. With a single predicate on the nullable side of a left join, nothing is pushed, so the query only fails with two or more predicates.

Fix

CollectionRef accepts an optional sourceId. The optimizer's copies, which are the same lexical source, keep the original identity, so the compiled subquery reads its own input and never falls back to alias resolution. cloneQueryForPlacement still mints fresh IDs on purpose (#1878).

Tests

New cases in tests/query/includes.test.ts under materialize:

  • Same alias: the included subquery reuses the join alias. Fails on main with [], passes with this change.
  • Distinct alias (control): passes on both.

A related optimizer issue showed up while debugging. It's fixed separately in #1977.

✅ 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 they pass.

🚀 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 live queries returning no rows in certain aliased subquery joins.
    • Included subqueries now read from the correct collection source, including when join aliases overlap.

The optimizer copied CollectionRefs with a fresh sourceId. Since TanStack#1877 the
compiler compiles the optimized subquery IR, so those copies no longer match
any input and fall back to an alias lookup. When an included subquery reuses
the alias, the join reads the included subquery's input and the live query
returns no rows.

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: 73bde869-3a05-472e-b676-81efb39697de

📥 Commits

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

📒 Files selected for processing (4)
  • .changeset/preserve-optimizer-source-ids.md
  • packages/db/src/query/ir.ts
  • packages/db/src/query/optimizer.ts
  • packages/db/tests/query/includes.test.ts

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


📝 Walkthrough

Walkthrough

The query optimizer now preserves collection source IDs when it replaces or copies collection references. Regression tests cover includes over parent subqueries with left joins and reused or distinct aliases.

Changes

Collection source identity

Layer / File(s) Summary
Preserve source IDs through optimization
packages/db/src/query/ir.ts, packages/db/src/query/optimizer.ts, packages/db/tests/query/includes.test.ts, .changeset/preserve-optimizer-source-ids.md
CollectionRef accepts an optional sourceId. Optimizer transformations preserve it. Regression tests cover includes over joined parent subqueries with reused and distinct aliases. The changeset records the patch.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: kyleamathews

Merge Risk: ⚪ Minimal · up to 6c032

The change preserves collection identity during optimization and adds regression coverage for materialized includes with reused aliases. No actionable merge-blocking risk is established; merge after normal checks pass.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6c032

The change restores correct source binding without showing new access to protected data. Normal query construction keeps independent sources separate. Low residual risk remains because callers can now supply identities explicitly, and externally constructed queries must preserve their uniqueness.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — Standard live-query and effect execution constructs separate graphs and source-keyed inputs from collections referenced by each query. The established affected scope is source selection and demand within those graphs; direct compiler consumers supply their own input maps.

Trust Boundaries and Controls

  • inferred — Application callers already control raw IR through the public compiler and builder constructor, including the input streams supplied to direct compilation. The new identity argument does not independently grant access to collections or credentials. No newly introduced less-trusted payload-to-privileged-stream path was established in the reviewed execution paths.

Resilience and Maintainability Implications

  • observed — The inspected runtime retains instance-scoped subscription ownership. Effect startup records release callbacks before lazy demand can fail and cleans up on startup failure. Live-query teardown releases callbacks and clears subscription and pipeline state so recovery creates fresh runtime inputs rather than reusing retired streams.

Hardening Proposals

  • proposed — Document supplied sourceId values as identity-preserving copies of the same logical source. If custom IR is accepted from less-trusted integrations, validate that distinct source placements cannot share an identity before deduplication and compilation.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly and concisely describes the main fix: preserving source identity in optimizer copies.
Description check ✅ Passed The description follows the repository template. It explains the change and cause, documents testing and known timeout behavior, and confirms that a changeset was generated.
Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ 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