feat: Subqueries in SELECT for hierarchical data (includes)#1294
feat: Subqueries in SELECT for hierarchical data (includes)#1294
Conversation
🦋 Changeset detectedLatest commit: fc269d5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
More templates
@tanstack/angular-db
@tanstack/db
@tanstack/db-ivm
@tanstack/electric-db-collection
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
|
Size Change: +3.38 kB (+3.65%) Total Size: 96 kB
ℹ️ View Unchanged
|
|
Size Change: 0 B Total Size: 3.7 kB ℹ️ View Unchanged
|
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kevin-dp
force-pushed
the
kevin/includes
branch
from
260157e to
d117fea
Compare
Replace O(n) parent collection scans with a reverse index (correlationKey → Set<parentKey>) for attaching child Collections to parent rows. The index is populated during parent INSERTs and cleaned up on parent DELETEs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
samwillis
left a comment
samwillis
left a comment
There was a problem hiding this comment.
This is looking really great. Awesome work!
Depending on if we are going to release before or after followup PRs it may make sense to add some defensive errors for unsupported queries (groupBy, referencing multiple fields on the parent)
| ? where.expression | ||
| : where | ||
|
|
||
| // Look for eq(a, b) where one side references parent and other references child |
There was a problem hiding this comment.
I believe this is finding the first expression that references both sides, this is correct. We should consider what something like this does:
q.from({ p: projects }).select(({ p }) => ({
id: p.id,
name: p.name,
issues: q
.from({ i: issues })
.where(({ i }) => and(eq(i.projectId, p.id)), eq(i.createdBy, p.createdBy))
.select(({ i }) => ({
id: i.id,
title: i.title,
})),
})),
)I suspect it breaks at the moment, and so we may want to throw if there is more than one expression matching both sources.
I think it's possible to make this work though by pulling the parent project value temporarily into the child issue pipeline.
There was a problem hiding this comment.
Indeed, it breaks right now because the parent row is not in the child pipeline. I added support for this in this PR: #1307
| // Re-add project Alpha — should get a fresh child collection | ||
| projects.utils.begin() | ||
| projects.utils.write({ | ||
| type: `insert`, | ||
| value: { id: 1, name: `Alpha Reborn` }, | ||
| }) | ||
| projects.utils.commit() | ||
|
|
||
| const alpha = collection.get(1) as any | ||
| expect(alpha).toMatchObject({ id: 1, name: `Alpha Reborn` }) | ||
| expect(childItems(alpha.issues)).toEqual([ | ||
| { id: 10, title: `Bug in Alpha` }, | ||
| { id: 11, title: `Feature for Alpha` }, | ||
| ]) |
|
ChatGPT review: Here’s my review of TanStack/db PR #1294 (adds “includes” subqueries / nested child collections). ([GitHub]1) What this PR is doing (as I understand it)
Overall: the API is very ergonomic, and the nested routing solution is clever. Things I like
Key concerns / suggested changes1) Alias collisions between parent and child queries (correctness bug risk)
Suggestion
2) Correlation extraction only matches top-level
|
|
@samwillis response to codex' review:
Valid concern but I don't think it's a real risk in practice. The child query is built via the builder API where the user explicitly declares aliases in
Fixed in #1307
This is a fair observation but the compiler already runs on the output of Still a valid code hygiene concern, but it's out of scope for this PR. If it were to be fixed, it should be its own cleanup.
The correlation field is almost always the parent's primary key (e.g., Could a user correlate on a non-PK field? Technically yes, but it would be semantically wrong — the correlation field determines how children are grouped to parents. If it's not stable, the entire grouping model is broken, not just the cleanup logic. I'd lean toward the first suggestion: document/validate that the correlation field should be a stable key (which it naturally is in every real use case). The "move" case handling would add complexity for a scenario that doesn't really make sense to support.
True — the correlation field doesn't have to be the PK (which are restricted to Using
This is fine. We have a couple of these reserved properties around the code. It's unlikely someone would use this name. Regarding the additional tests:
|
…hIncludesState reads from that stamp. The stamp is cleaned up at the end of flush so it never leaks to the user
2 participants
Summary
.select()that produce hierarchical results — each parent row gets a child Collection (e.g., projects with nested issues, issues with nested comments)Closes #288
Test plan
🤖 Generated with Claude Code