fix: correlated subquery resolves empty when correlation targets a joined alias - #1705
fix: correlated subquery resolves empty when correlation targets a joined alias#1705Mohith26 wants to merge 2 commits into
Conversation
…ined alias The includes-child compilation re-keyed the raw main-source input by childCorrelationField.path.slice(1), silently assuming the correlation ref lives on the subquery's own from alias. When it references a joined alias instead, every correlation value evaluated to undefined and the inner join with parent keys dropped all rows, so materialize() resolved to a permanently empty array. Defer the parent-key filter until after joins in that case: re-key the namespaced pipeline by the compiled correlation expression, inner-join with the parent key stream, and tag __correlationKey on the main source namespace (with parent context merged) so downstream routing, group-by, order-by and output extraction keep working unchanged. Fixes TanStack#1704
Covers the initial resolve, incremental insert/delete propagation, the issue's exact spread-select shape, and a control asserting from-alias correlation still works when the subquery contains a join.
📝 WalkthroughWalkthrough
ChangesMaterialize correlation handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
packages/db/tests/query/includes.test.ts (2)
6794-6818: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise incremental changes to the joined correlation source.
These assertions only insert and delete
comments. Also update or delete anissuesrow so itsprojectIdchanges, then verify the comment is removed from one project and routed to the other. That directly protects the new post-join correlation path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/tests/query/includes.test.ts` around lines 6794 - 6818, Extend the incremental joined-correlation test around collection.get(1) to update or delete an issues row and change its projectId, then verify the associated comment is removed from the original project and appears under the destination project. Keep the existing comment insert/delete assertions and exercise the post-join routing path through an issues-source change.
6780-6780: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the regression assertions type-checked.
Replace these
as anycasts with non-null assertions or explicit expected result types so schema/result inference regressions remain visible to TypeScript.As per coding guidelines: “Avoid using
anytypes; useunknowninstead when the type is truly unknown.” <coding_guidelines>Also applies to: 6786-6786, 6791-6792, 6802-6802, 6815-6815, 6837-6837, 6857-6857
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/tests/query/includes.test.ts` at line 6780, Remove the as any casts from the regression assertions around collection.get calls, including the related lines, and preserve type checking by using non-null assertions or explicit expected result types. Keep the existing assertions and test behavior unchanged while ensuring schema/result inference errors remain visible to TypeScript.Source: Coding guidelines
packages/db/src/query/compiler/index.ts (2)
326-373: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared correlation filtering pipeline.
This duplicates the pre-join rekey/join/filter/effective-key flow at Lines 262-293. Extract the common mechanics with callbacks for reading and tagging each row shape so both paths cannot drift.
As per coding guidelines: “Extract common logic into utility functions when identical or near-identical code blocks appear in multiple places.” <coding_guidelines>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/query/compiler/index.ts` around lines 326 - 373, Extract the shared rekey, inner-join, filtering, correlation tagging, parent-context merge, and effective-key generation from this block and the corresponding pre-join flow into a reusable helper. Parameterize the helper with callbacks for reading correlation values and tagging each row shape, then update both paths around compileExpression(childCorrelationField!) to use it while preserving their existing outputs.Source: Coding guidelines
330-371: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the new
anyannotations with typed stream tuples.Define types for the namespaced row and joined values, then annotate the callbacks and returned key-value tuples precisely. This preserves compiler checks around
childSide,parentSide, andnamespacedRow[mainSource].As per coding guidelines: “Avoid using
anytypes; useunknowninstead when the type is truly unknown, and provide proper type annotations for return values.” <coding_guidelines>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/query/compiler/index.ts` around lines 330 - 371, Replace the any annotations in the rekeying, filtering, and mapping callbacks with explicit types for namespaced rows, child tuples, parent context, and joined stream values. Define reusable tuple/object types near this join logic, type namespacedRow[mainSource] safely, and annotate the returned effective key/tagged-row tuple without weakening checks around childSide or parentSide.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/db/src/query/compiler/index.ts`:
- Around line 326-373: Extract the shared rekey, inner-join, filtering,
correlation tagging, parent-context merge, and effective-key generation from
this block and the corresponding pre-join flow into a reusable helper.
Parameterize the helper with callbacks for reading correlation values and
tagging each row shape, then update both paths around
compileExpression(childCorrelationField!) to use it while preserving their
existing outputs.
- Around line 330-371: Replace the any annotations in the rekeying, filtering,
and mapping callbacks with explicit types for namespaced rows, child tuples,
parent context, and joined stream values. Define reusable tuple/object types
near this join logic, type namespacedRow[mainSource] safely, and annotate the
returned effective key/tagged-row tuple without weakening checks around
childSide or parentSide.
In `@packages/db/tests/query/includes.test.ts`:
- Around line 6794-6818: Extend the incremental joined-correlation test around
collection.get(1) to update or delete an issues row and change its projectId,
then verify the associated comment is removed from the original project and
appears under the destination project. Keep the existing comment insert/delete
assertions and exercise the post-join routing path through an issues-source
change.
- Line 6780: Remove the as any casts from the regression assertions around
collection.get calls, including the related lines, and preserve type checking by
using non-null assertions or explicit expected result types. Keep the existing
assertions and test behavior unchanged while ensuring schema/result inference
errors remain visible to TypeScript.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b89689c-402c-4419-b219-aebf98edaed2
📒 Files selected for processing (3)
.changeset/plenty-hoops-shake.mdpackages/db/src/query/compiler/index.tspackages/db/tests/query/includes.test.ts
1 participant
Fixes #1704, reported by @Jaime02.
In
compileQuery, the includes-child path re-keys the raw main-source input bychildCorrelationField.path.slice(1), assuming the correlation ref lives on the subquery's ownfromalias. When it references a joined alias instead (eq(order.partId, part.id)withfrom(production).innerJoin(order)), the nested lookup returnsundefined, so the inner join with parent keys drops every row: the subquery resolves to[]permanently, with no error.The fix defers the parent-key filter when the correlation alias is not the main source: the identical inner join is applied after
processJoinsagainst the namespaced pipeline using the compiled correlation expression, tagging__correlationKeyon the main-source namespace and merging parent context so group-by, order-by, and output routing consume it unchanged. Lazy loading needed no change (followRefalready resolves joined aliases). Patch changeset included.Two regression tests cover initial resolution, incremental insert/delete propagation, the issue's exact spread-select shape, and a from-alias-with-join control; both fail without the fix. Package suite: 2485 passed, zero new failures; eslint, build, and vitest typecheck clean.
Summary by CodeRabbit
materialize()returning empty results when correlation conditions referenced fields from joined data.