feat(db): support multi-source from queries#1537
Open
samwillis wants to merge 35 commits into
Open
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
📝 WalkthroughWalkthroughThis PR implements two major query-building features: ChangescaseWhen and unionAll Query Features
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
|
Thread conditional include destinations through caseWhen branches and tighten aggregate handling for grouped conditional projections. Co-authored-by: Cursor <cursoragent@cursor.com>
samwillis
force-pushed
the
multi-source-from
branch
from
a79e5eb to
7fa5b53
Compare
More templates
@tanstack/angular-db
@tanstack/browser-db-sqlite-persistence
@tanstack/capacitor-db-sqlite-persistence
@tanstack/cloudflare-durable-objects-db-sqlite-persistence
@tanstack/db
@tanstack/db-ivm
@tanstack/db-sqlite-persistence-core
@tanstack/electric-db-collection
@tanstack/electron-db-sqlite-persistence
@tanstack/expo-db-sqlite-persistence
@tanstack/node-db-sqlite-persistence
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/react-native-db-sqlite-persistence
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/tauri-db-sqlite-persistence
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
Contributor
|
Size Change: +2.52 kB (+2.15%) Total Size: 119 kB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
|
Size Change: 0 B Total Size: 4.24 kB ℹ️ View Unchanged
|
Contributor
|
@samwillis I find the multi from confusing. There is no SQL equivalent, the closest you can get in SQL is to const feed = createLiveQueryCollection((q) =>
q
.from({
message: messages,
toolCall: toolCalls,
})
.orderBy(({ message, toolCall }) =>
coalesce(message.timestamp, toolCall.timestamp),
),
)I find this query confusing because i initially thought that this is just another notation for q
.from({
message: messages,
})
.orderBy(({ message }) =>
message.timestamp
)
UNION
q
.from({
toolCall: toolCalls,
})
.orderBy(({ toolCall }) =>
toolCall.timestamp
)But it is not equivalent to this because SELECT * FROM (
SELECT m.* AS message, NULL AS toolCall FROM messages m
UNION ALL
SELECT NULL AS message, t.* AS toolCall FROM toolCalls t
) sub
ORDER BY coalesce(sub.message.timestamp, sub.toolCall.timestamp)I find this very confusing. You can't tell that from reading With explicit q.unionAll(
q.from({ message: messages }),
q.from({ toolCall: toolCalls }),
).orderBy(({ message, toolCall }) =>
coalesce(message.timestamp, toolCall.timestamp),
) // obviously post-unionCompare this to the current form where you have to infer scope from the shape of the |
Use one compiler helper for scalar and projection caseWhen branch truthiness so every evaluation path stays consistent. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep precise caseWhen typings to five branches to reduce API surface while preserving runtime behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
Preserve include nodes during aggregate extraction and handle null projection branches without misclassifying conditional selects. Co-authored-by: Cursor <cursoragent@cursor.com>
Rewrite grouped refs inside aggregate-wrapped conditional projections so branch conditions and values evaluate against grouped keys. Co-authored-by: Cursor <cursoragent@cursor.com>
Allow longer caseWhen calls without reintroducing verbose precise overloads past five branches. Co-authored-by: Cursor <cursoragent@cursor.com>
Avoid treating projection objects with type fields as IR expressions when selecting conditional projections. Co-authored-by: Cursor <cursoragent@cursor.com>
Add union-style from handling across the query builder, compiler, optimizer, live traversal, and tests so independent sources can feed one live collection. Co-authored-by: Cursor <cursoragent@cursor.com>
Tighten union join typing, avoid inactive guarded include evaluation, and document multi-source ordering defaults. Co-authored-by: Cursor <cursoragent@cursor.com>
Add runtime and type coverage for union branches that are joined and projected subqueries, since that is the main pre-filtering pattern. Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve multi-source subquery join targets per branch so safe coalesce projections can load indexed subsets while computed or limited branches deopt to eager loading. Co-authored-by: Cursor <cursoragent@cursor.com>
Share union-aware lazy target resolution between joins and includes so correlated child collections can be built from subquery or multi-source sources without crashing. Co-authored-by: Cursor <cursoragent@cursor.com>
Carry nested include routing through QueryRef sources and preserve keyed incremental updates so child live queries receive streaming materialization changes. Co-authored-by: Cursor <cursoragent@cursor.com>
Tighten branch key encoding, join pushdown nullability, lazy target resolution, and conditional/grouped projection semantics based on review feedback. Co-authored-by: Cursor <cursoragent@cursor.com>
Restore nested update cloning after rebasing onto the latest caseWhen branch and update conditional include expectations. Co-authored-by: Cursor <cursoragent@cursor.com>
Assert subquery-backed join lazy loading marks the concrete subscribed alias rather than the outer QueryRef alias. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
samwillis
force-pushed
the
multi-source-from
branch
from
6bb8cc8 to
084a4a5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (10)
docs/guides/live-queries.md (1)
484-540: ⚡ Quick winConsider adding guidance on when to use
unionAll()vsfrom().The "From Clause" section describes single-source
from()usage, and the following "Source-LevelunionAll" section describes combining multiple sources. However, there's no explicit guidance on when developers should choose one over the other, or whether multi-sourcefrom()is still supported.Given that the UNION_ALL_REFACTOR_PLAN.md indicates multi-source
from()will be deprecated, consider adding a note like:> **Note:** Use `from()` with a single source. To combine multiple independent sources, use `unionAll()` instead (see [Source-Level unionAll](`#source-level-unionall`) below).This helps developers understand the intended usage pattern and prevents confusion about the API design.
🤖 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 `@docs/guides/live-queries.md` around lines 484 - 540, Add a short advisory note in the "From Clause" section clarifying that from() should be used with a single source and that combining multiple independent sources should be done with unionAll() (reference the Source-Level unionAll section), and mention that multi-source from() will be deprecated per the UNION_ALL_REFACTOR_PLAN.md; update the text near the Method Signature or Basic Usage (where from({ [alias]: Collection | Query }) and examples appear) to include this guidance and a link/reference to the Source-Level unionAll section.packages/db/src/errors.ts (1)
389-402: ⚡ Quick winConsider using a union type for the
contextparameter.The
contextparameter is checked using a string literal (context === "unionAll clause"). Using a typed union would provide compile-time safety and prevent typos:constructor(context: "from clause" | "unionAll clause" | "join clause", type: string)This ensures callers pass valid context strings and makes the valid values discoverable through IDE autocomplete.
🤖 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/errors.ts` around lines 389 - 402, The constructor in errors.ts currently accepts an untyped string for context and relies on string-equality checks (e.g., context === "unionAll clause"); change the constructor signature to use a string literal union for allowed contexts (e.g., "from clause" | "unionAll clause" | "join clause") so the compiler enforces valid values, update any internal checks that compare context to those literals to remain the same, and update all call sites of this constructor to pass one of the unioned literal values so callers get IDE autocomplete and type safety (identify the constructor in this file to make the signature change and scan for usages of this error class to update callers).UNION_ALL_REFACTOR_PLAN.md (1)
1-235: 💤 Low valueConsider moving this planning document to a more appropriate location.
This refactor plan is currently in the repository root. Planning and design documents are typically better organized in dedicated directories. Consider moving it to:
docs/design/UNION_ALL_REFACTOR_PLAN.md(if you have design docs).github/docs/UNION_ALL_REFACTOR_PLAN.md(for GitHub-specific planning)- Or a dedicated
planning/directoryThis keeps the repo root cleaner and makes it easier to find design documentation alongside other architectural decisions.
🤖 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 `@UNION_ALL_REFACTOR_PLAN.md` around lines 1 - 235, Move the planning doc "UNION_ALL_REFACTOR_PLAN.md" out of the repo root into a documentation folder (suggested targets: docs/design/UNION_ALL_REFACTOR_PLAN.md, .github/docs/UNION_ALL_REFACTOR_PLAN.md, or planning/UNION_ALL_REFACTOR_PLAN.md), update any references to the file (README, docs index, or issue/PR descriptions) to the new path, and ensure the top-level header "# `unionAll` Query Builder Refactor Plan" remains unchanged so searches still find it; create or update a docs/index or TOC entry if you have one so the plan is discoverable after the move.packages/db/tests/query/case-when.test.ts (3)
79-769: ⚡ Quick winAdd explicit empty/single-element case coverage for
caseWhenqueries.This suite is comprehensive, but it’s missing direct edge-case assertions for empty collections and single-element inputs on conditional projection/include paths. A couple of targeted tests here would harden regressions around null/default materialization and grouped behavior.
As per coding guidelines,
Test corner cases including: empty collections, single elements, undefined vs null, resolved promises, race conditions, limit/offset edge cases.🤖 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/case-when.test.ts` around lines 79 - 769, Add targeted tests covering empty and single-element inputs for caseWhen paths: add at least two tests using createCollection/createLiveQueryCollection + preload that assert behavior when the source collection is empty and when it contains a single item (verify conditional scalar branches, projection objects, includes/toArray branches and null vs undefined handling). Reference the existing patterns in this file (functions/symbols: caseWhen, createLiveQueryCollection, createCollection, createUsersCollection/createPostsCollection, preload, toArray, orderBy, stripVirtualPropsAndSymbols) and follow the existing test structure and expectations to assert correct materialization for empty results (e.g. null/default branches, empty arrays for includes) and single-element results. Ensure tests cover both scalar and object branches, and grouped/aggregated usages where applicable.
59-77: ⚡ Quick winReplace
anyin helper utilities withunknown+ narrowing.Line 59/65/75/76 currently erase type checks with
any. Please switch tounknownand narrow locally to keep helper behavior type-safe.Proposed patch
-function stripVirtualPropsAndSymbols(value: any): any { +function stripVirtualPropsAndSymbols(value: unknown): unknown { if (Array.isArray(value)) { return value.map((entry) => stripVirtualPropsAndSymbols(entry)) } if (value && typeof value === `object`) { - const out: Record<string, any> = {} + const out: Record<string, unknown> = {} for (const [key, entry] of Object.entries(stripVirtualProps(value))) { out[key] = stripVirtualPropsAndSymbols(entry) } return out } return value } -function childRows(collection: any): Array<any> { - return [...collection.toArray].map((row) => stripVirtualPropsAndSymbols(row)) +function childRows(collection: { toArray: ReadonlyArray<unknown> }): Array<unknown> { + return collection.toArray.map((row) => stripVirtualPropsAndSymbols(row)) }As per coding guidelines,
Avoid using any types; use unknown instead when type is truly unknown and provide type guards to narrow safely.🤖 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/case-when.test.ts` around lines 59 - 77, Replace the use of `any` in the helpers by declaring parameters and intermediate vars as `unknown` and adding local narrowing guards: in stripVirtualPropsAndSymbols change the parameter type to `unknown`, use Array.isArray to narrow arrays, check `value !== null && typeof value === 'object'` and index into Object.entries only after that check (and preserve use of stripVirtualProps by narrowing its input), and in childRows type the `collection` parameter as `unknown` then assert/guard that it has a `toArray` iterable (e.g., check `collection && typeof (collection as any).toArray !== 'undefined'`) before spreading and mapping; update all local variables similarly so no flow uses `any` without narrowing. Ensure references to the functions stripVirtualPropsAndSymbols and childRows remain unchanged.
39-57: ⚡ Quick winAnnotate return types for collection factory helpers.
createUsersCollectionandcreatePostsCollectionshould declare explicit return types to avoid inference drift in tests that depend on these helpers.As per coding guidelines,
Provide proper type annotations for function return values instead of relying on implicit types.🤖 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/case-when.test.ts` around lines 39 - 57, The two test helper functions createUsersCollection and createPostsCollection lack explicit return type annotations which can cause inference drift; add an explicit return type to each (for example ": ReturnType<typeof createCollection>" or the concrete collection type used in your codebase such as "SyncCollection<User>" / "SyncCollection<Post>") to their declarations, e.g. function createUsersCollection(): ReturnType<typeof createCollection> { ... } and function createPostsCollection(): ReturnType<typeof createCollection> { ... }, and add any necessary imports for the concrete type if you choose not to use ReturnType.packages/db/tests/query/case-when.test-d.ts (1)
31-49: ⚡ Quick winAdd explicit return types to test helper factories.
createUsersandcreatePostscurrently rely on inference. Please annotate return types so helper contracts remain stable ifcreateCollectioninference changes.As per coding guidelines,
Provide proper type annotations for function return values instead of relying on implicit types.🤖 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/case-when.test-d.ts` around lines 31 - 49, The test helper functions createUsers and createPosts lack explicit return type annotations; update their signatures to declare the concrete return type returned by createCollection (e.g., the collection type produced when calling createCollection with mockSyncCollectionOptions<User> / mockSyncCollectionOptions<Post>) so the helpers' contracts do not depend on inference from createCollection or mockSyncCollectionOptions. Locate the createUsers and createPosts functions and add the appropriate return type annotations matching the collection type they produce.packages/db/tests/query/builder/union-all.test.ts (1)
46-47: ⚡ Quick winAvoid
as anyin IR assertions; narrow by discriminant instead.These casts suppress the exact type guarantees this builder test should enforce.
♻️ Proposed assertion tightening
- expect( - (builtQuery.from as any).sources.map((source: any) => source.alias), - ).toEqual([`employees`, `departments`]) + if (builtQuery.from.type !== `unionFrom`) { + throw new Error(`Expected unionFrom`) + } + expect(builtQuery.from.sources.map((source) => source.alias)).toEqual([ + `employees`, + `departments`, + ]) @@ - expect((builtQuery.from as any).queries).toHaveLength(2) + if (builtQuery.from.type !== `unionAll`) { + throw new Error(`Expected unionAll`) + } + expect(builtQuery.from.queries).toHaveLength(2)As per coding guidelines,
**/*.{ts,tsx}: "Avoid usinganytypes; useunknowninstead when type is truly unknown and provide type guards to narrow safely".Also applies to: 81-81
🤖 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/builder/union-all.test.ts` around lines 46 - 47, The test currently uses (builtQuery.from as any).sources to bypass type checking; instead, narrow the union by checking the discriminant on each source before accessing alias. Update the assertion to iterate builtQuery.from.sources, use a type guard or an inline check like if (source.type === 'table' || source.kind === 'table') to narrow the type, then collect source.alias values and expect them toEqual([`employees`,`departments`]); apply the same pattern for the other occurrence around line 81 to remove `as any` casts and rely on explicit discriminant narrowing.packages/db/tests/query/union-all.test.ts (1)
182-198: ⚡ Quick winReplace
anyin test helpers/callbacks withunknown+ narrowing.These
anyannotations weaken the exact safety guarantees this union-heavy suite is meant to protect.♻️ Proposed typed helper refactor
-function stripVirtualPropsDeep(value: any): any { +function isRecord(value: unknown): value is Record<string, unknown> { + return value !== null && typeof value === `object` +} + +function stripVirtualPropsDeep(value: unknown): unknown { if (Array.isArray(value)) { return value.map((entry) => stripVirtualPropsDeep(entry)) } - if (value && typeof value === `object`) { - const out: Record<string, any> = {} + if (isRecord(value)) { + const out: Record<string, unknown> = {} for (const [key, entry] of Object.entries(stripVirtualProps(value))) { out[key] = stripVirtualPropsDeep(entry) } return out } return value } -function childRows(collection: any): Array<any> { +function childRows<T>(collection: { toArray: Array<T> }): Array<unknown> { return [...collection.toArray].map((row) => stripVirtualPropsDeep(row)) }As per coding guidelines,
**/*.{ts,tsx}: "Avoid usinganytypes; useunknowninstead when type is truly unknown and provide type guards to narrow safely".Also applies to: 595-597, 619-621, 713-715, 1148-1158, 1431-1450
🤖 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/union-all.test.ts` around lines 182 - 198, Replace the loose any types in the test helpers by using unknown and explicit narrowing: change function signatures stripVirtualPropsDeep(value: any) -> stripVirtualPropsDeep(value: unknown) and childRows(collection: unknown) -> childRows(collection: { toArray: unknown } | unknown), then implement safe type-guards (Array.isArray, typeof value === "object" && value !== null) and use Record<string, unknown> for object outputs; call stripVirtualProps(value) expecting unknown and narrow its result to Record<string, unknown> before iterating entries, recursively call stripVirtualPropsDeep with narrowed types, and in childRows assert collection has a toArray that is an iterable and map its elements after narrowing—apply the same unknown-to-narrowing pattern to the other helpers mentioned (lines ~595-597, 619-621, 713-715, 1148-1158, 1431-1450).packages/db/src/query/compiler/evaluators.ts (1)
577-595: ⚡ Quick winUse
unknownfor the new exported predicate helper.
anyhere disables checking for every caller of this helper.unknownkeeps the implementation the same while preserving safe narrowing at the call sites.♻️ Proposed fix
-export function isCaseWhenConditionTrue(value: any): boolean { +export function isCaseWhenConditionTrue(value: unknown): boolean { if (value == null || value === false) { return false }As per coding guidelines, "Avoid using
anytypes; useunknowninstead when type is truly unknown and provide type guards to narrow safely".🤖 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/evaluators.ts` around lines 577 - 595, Change the exported predicate helper isCaseWhenConditionTrue to accept value: unknown instead of any to preserve caller type-safety; update the function signature to isCaseWhenConditionTrue(value: unknown): boolean and keep the existing runtime checks (null/false, true, typeof number/bigint, Boolean(value)) so behavior is unchanged while callers retain safe narrowing.
🤖 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.
Inline comments:
In `@packages/db/src/query/builder/functions.ts`:
- Around line 570-596: The caseWhen overload that accepts a 6th condition/value
pair (the function signature beginning with "export function caseWhen< C1...V5
>( ... condition6: ExpressionLike, value6: CaseWhenValue, ...rest:
Array<CaseWhenValue> ): any") currently returns any and loses type information;
update the overloads so they return a properly typed CaseWhenResult (e.g.,
CaseWhenResult<C1,V1,C2,V2,C3,V3,C4,V4,C5,V5,C6,V6> or the appropriate generic
for the six-pair form), and add the matching overload for the six-pair plus
default value variant (or replace with a variadic-tuple generic that preserves
exact types for all arities). Ensure you reference the existing types
ExpressionLike, CaseWhenValue and CaseWhenResult and keep the runtime
implementation signature unchanged (only adjust the TypeScript overload
signatures) so downstream query types retain full type inference.
- Around line 71-81: The conditional in the CaseWhenResult type is distributing
over union members causing mixed branch unions to be typed as a union of
BasicExpression and CaseWhenWrapper; change the distributive check to a
non-distributive form so the type only yields BasicExpression when all branch
values are ExpressionLike. Concretely, replace "TValues[number] extends
ExpressionLike" with "[TValues[number]] extends [ExpressionLike]" (and keep the
THasDefault logic intact) so CaseWhenResult correctly resolves to
BasicExpression only when every branch is expression-like and to CaseWhenWrapper
otherwise; update the type use-sites (e.g., where CaseWhenResult is returned
from caseWhen) to rely on this corrected behavior.
In `@packages/db/src/query/builder/index.ts`:
- Around line 1126-1129: The fallback that builds childAliases only handles
childQuery.from.type === `unionFrom` and uses sources, but misses the `unionAll`
case so branch aliases (e.g., branchA/branchB) are not included and
extractCorrelation never matches those refs; update the logic around
childAliases to also detect `childQuery.from.type === 'unionAll'` and collect
all branch aliases (e.g., map over childQuery.from.branches or equivalent branch
list to extract each branch.alias) so extractCorrelation will see aliases from
unionAll branches as well.
In `@packages/db/src/query/builder/types.ts`:
- Around line 590-595: The ValueOfUnion helper currently returns never when a
union branch lacks key K which makes branch-exclusive fields appear required;
update ValueOfUnion (the conditional type) so that when K is not a key of a
branch it yields undefined instead of never, preserving undefined for missing
keys and making fields like toolCall optional in unions used by RefsForContext;
locate the ValueOfUnion type in types.ts and change the branch that now produces
never to produce undefined while keeping the existing conditional structure.
In `@packages/db/src/query/compiler/group-by.ts`:
- Around line 919-939: compileGroupedConditionalSelect currently returns
undefined when no branch matches and no defaultValue is provided; change it to
return null to match non-grouped behavior. Locate
compileGroupedConditionalSelect and adjust the final return so that if
defaultValue is undefined it returns null, otherwise it invokes and returns
defaultValue(row); keep branch evaluation logic
(branches.map/isCaseWhenConditionTrue) unchanged. Ensure you reference the
compiled defaultValue (the result of compileGroupedSelectValue) rather than the
original conditional.defaultValue when deciding to return null.
In `@packages/db/src/query/compiler/index.ts`:
- Around line 440-446: The current clone only shallow-copies query.select but
replaceIncludesInSelect() mutates nested branches (including ConditionalSelect),
which modifies the shared IR; change the flow so replacements are done
immutably: update or replace replaceIncludesInSelect to return a new transformed
Select tree (or perform path-copying on any node it touches) instead of mutating
nodes in place, and invoke it to assign the returned select to query.select;
also apply the same immutable transformation approach to the other occurrence
referenced around the 1878-1933 region to ensure no nested objects are mutated.
- Around line 244-245: The union-backed include path is skipping the
parentKeyStream when isUnionFrom is true so processFromClause/processUnionAll
never receive the correlation stream and children never get __correlationKey;
update the logic around parentKeyStream/isUnionFrom (where mainInput is selected
from sources[mainSource]) to always propagate the parentKeyStream (and attach
childCorrelationField) into the per-branch processing: pass the correlation
stream into processFromClause and processUnionAll for each union branch, ensure
__correlationKey is set on union branch inputs (not just non-union paths), and
update any callers that build the union input so that union-backed includes
compile/load the filtered, correlated subset and can be routed back to parent
rows.
In `@packages/db/src/query/compiler/lazy-targets.ts`:
- Around line 122-166: getSourceFromAlias currently only looks at the top-level
from and misses aliases defined on query joins or inside queryRef/union
branches, causing getTargetsFromPropRef to miss valid join-backed aliases;
update getSourceFromAlias (and usages in
getTargetsFromPropRef/getTargetsFromQueryRef) to collect and search aliases
from: the top-level from sources, any nested QueryRef sources' query.from and
query.join arrays (and for unionFrom/unionAll iterate each branch's from and its
query joins), so that when a matching alias exists in a join or inside a
queryRef branch you return the correct CollectionRef or QueryRef instead of
undefined. Ensure you still respect existing branch logic (e.g., unionAll -> [])
and preserve source.type checks like collectionRef vs queryRef.
In `@packages/db/src/query/live/utils.ts`:
- Around line 118-121: extractCollectionFromSource currently returns the first
branch for unionFrom/unionAll which lets CollectionConfigBuilder implicitly
inherit defaultStringCollation from an arbitrary branch; instead, update
extractCollectionFromSource (or CollectionConfigBuilder's use of it) to either
validate that all branches in unionFrom/unionAll share the same
compareOptions/defaultStringCollation and throw if they differ, or require
callers to pass an explicit defaultStringCollation for union queries;
specifically, when encountering from.type === 'unionFrom' or 'unionAll', iterate
all sources/queries and compare their compareOptions/defaultStringCollation
values, and if any mismatch is detected raise an error (or clear the inherited
collation and force an explicit parameter) so collation is not silently taken
from the first branch.
In `@packages/db/src/query/optimizer.ts`:
- Around line 908-909: getFirstFromAlias currently dereferences
getFromSources(...)[0] which throws for unionAll (no sources); update
getFirstFromAlias to guard for an empty array (or query.kind === 'unionAll') and
return a safe sentinel (e.g. empty string) instead of indexing into [0]; also
apply the same guard to the similar code paths around the
referencesAliasWithRemappedSelect call (the occurrence at lines ~1194-1197) so
callers receive a safe value and the optimizer will conservatively avoid
pushdown instead of crashing.
In `@packages/db/tests/query/case-when.test-d.ts`:
- Around line 127-324: Tests and types disagree on whether non-matching caseWhen
branches are represented as undefined or null; fix by making the representation
consistent. Update the caseWhen implementation and its type signature (the
function named caseWhen) to return null for non-matching branches (or, if you
prefer the opposite, update the runtime to return undefined) and then align all
type tests (references: adultProfile, postProfile, profile, maybeProfile, and
any caseWhen usages in these tests) to expect that same nullable value (replace
| undefined with | null across the failing expectTypeOf assertions) so runtime,
types, and docs all match.
---
Nitpick comments:
In `@docs/guides/live-queries.md`:
- Around line 484-540: Add a short advisory note in the "From Clause" section
clarifying that from() should be used with a single source and that combining
multiple independent sources should be done with unionAll() (reference the
Source-Level unionAll section), and mention that multi-source from() will be
deprecated per the UNION_ALL_REFACTOR_PLAN.md; update the text near the Method
Signature or Basic Usage (where from({ [alias]: Collection | Query }) and
examples appear) to include this guidance and a link/reference to the
Source-Level unionAll section.
In `@packages/db/src/errors.ts`:
- Around line 389-402: The constructor in errors.ts currently accepts an untyped
string for context and relies on string-equality checks (e.g., context ===
"unionAll clause"); change the constructor signature to use a string literal
union for allowed contexts (e.g., "from clause" | "unionAll clause" | "join
clause") so the compiler enforces valid values, update any internal checks that
compare context to those literals to remain the same, and update all call sites
of this constructor to pass one of the unioned literal values so callers get IDE
autocomplete and type safety (identify the constructor in this file to make the
signature change and scan for usages of this error class to update callers).
In `@packages/db/src/query/compiler/evaluators.ts`:
- Around line 577-595: Change the exported predicate helper
isCaseWhenConditionTrue to accept value: unknown instead of any to preserve
caller type-safety; update the function signature to
isCaseWhenConditionTrue(value: unknown): boolean and keep the existing runtime
checks (null/false, true, typeof number/bigint, Boolean(value)) so behavior is
unchanged while callers retain safe narrowing.
In `@packages/db/tests/query/builder/union-all.test.ts`:
- Around line 46-47: The test currently uses (builtQuery.from as any).sources to
bypass type checking; instead, narrow the union by checking the discriminant on
each source before accessing alias. Update the assertion to iterate
builtQuery.from.sources, use a type guard or an inline check like if
(source.type === 'table' || source.kind === 'table') to narrow the type, then
collect source.alias values and expect them
toEqual([`employees`,`departments`]); apply the same pattern for the other
occurrence around line 81 to remove `as any` casts and rely on explicit
discriminant narrowing.
In `@packages/db/tests/query/case-when.test-d.ts`:
- Around line 31-49: The test helper functions createUsers and createPosts lack
explicit return type annotations; update their signatures to declare the
concrete return type returned by createCollection (e.g., the collection type
produced when calling createCollection with mockSyncCollectionOptions<User> /
mockSyncCollectionOptions<Post>) so the helpers' contracts do not depend on
inference from createCollection or mockSyncCollectionOptions. Locate the
createUsers and createPosts functions and add the appropriate return type
annotations matching the collection type they produce.
In `@packages/db/tests/query/case-when.test.ts`:
- Around line 79-769: Add targeted tests covering empty and single-element
inputs for caseWhen paths: add at least two tests using
createCollection/createLiveQueryCollection + preload that assert behavior when
the source collection is empty and when it contains a single item (verify
conditional scalar branches, projection objects, includes/toArray branches and
null vs undefined handling). Reference the existing patterns in this file
(functions/symbols: caseWhen, createLiveQueryCollection, createCollection,
createUsersCollection/createPostsCollection, preload, toArray, orderBy,
stripVirtualPropsAndSymbols) and follow the existing test structure and
expectations to assert correct materialization for empty results (e.g.
null/default branches, empty arrays for includes) and single-element results.
Ensure tests cover both scalar and object branches, and grouped/aggregated
usages where applicable.
- Around line 59-77: Replace the use of `any` in the helpers by declaring
parameters and intermediate vars as `unknown` and adding local narrowing guards:
in stripVirtualPropsAndSymbols change the parameter type to `unknown`, use
Array.isArray to narrow arrays, check `value !== null && typeof value ===
'object'` and index into Object.entries only after that check (and preserve use
of stripVirtualProps by narrowing its input), and in childRows type the
`collection` parameter as `unknown` then assert/guard that it has a `toArray`
iterable (e.g., check `collection && typeof (collection as any).toArray !==
'undefined'`) before spreading and mapping; update all local variables similarly
so no flow uses `any` without narrowing. Ensure references to the functions
stripVirtualPropsAndSymbols and childRows remain unchanged.
- Around line 39-57: The two test helper functions createUsersCollection and
createPostsCollection lack explicit return type annotations which can cause
inference drift; add an explicit return type to each (for example ":
ReturnType<typeof createCollection>" or the concrete collection type used in
your codebase such as "SyncCollection<User>" / "SyncCollection<Post>") to their
declarations, e.g. function createUsersCollection(): ReturnType<typeof
createCollection> { ... } and function createPostsCollection():
ReturnType<typeof createCollection> { ... }, and add any necessary imports for
the concrete type if you choose not to use ReturnType.
In `@packages/db/tests/query/union-all.test.ts`:
- Around line 182-198: Replace the loose any types in the test helpers by using
unknown and explicit narrowing: change function signatures
stripVirtualPropsDeep(value: any) -> stripVirtualPropsDeep(value: unknown) and
childRows(collection: unknown) -> childRows(collection: { toArray: unknown } |
unknown), then implement safe type-guards (Array.isArray, typeof value ===
"object" && value !== null) and use Record<string, unknown> for object outputs;
call stripVirtualProps(value) expecting unknown and narrow its result to
Record<string, unknown> before iterating entries, recursively call
stripVirtualPropsDeep with narrowed types, and in childRows assert collection
has a toArray that is an iterable and map its elements after narrowing—apply the
same unknown-to-narrowing pattern to the other helpers mentioned (lines
~595-597, 619-621, 713-715, 1148-1158, 1431-1450).
In `@UNION_ALL_REFACTOR_PLAN.md`:
- Around line 1-235: Move the planning doc "UNION_ALL_REFACTOR_PLAN.md" out of
the repo root into a documentation folder (suggested targets:
docs/design/UNION_ALL_REFACTOR_PLAN.md, .github/docs/UNION_ALL_REFACTOR_PLAN.md,
or planning/UNION_ALL_REFACTOR_PLAN.md), update any references to the file
(README, docs index, or issue/PR descriptions) to the new path, and ensure the
top-level header "# `unionAll` Query Builder Refactor Plan" remains unchanged so
searches still find it; create or update a docs/index or TOC entry if you have
one so the plan is discoverable after the move.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a8c3e0df-e8de-477a-9339-ee31fb42af1c
📒 Files selected for processing (31)
.changeset/tender-mugs-hear.mdUNION_ALL_REFACTOR_PLAN.mddocs/guides/live-queries.mdpackages/db/src/errors.tspackages/db/src/query/builder/functions.tspackages/db/src/query/builder/index.tspackages/db/src/query/builder/ref-proxy.tspackages/db/src/query/builder/types.tspackages/db/src/query/compiler/evaluators.tspackages/db/src/query/compiler/group-by.tspackages/db/src/query/compiler/index.tspackages/db/src/query/compiler/joins.tspackages/db/src/query/compiler/lazy-targets.tspackages/db/src/query/compiler/order-by.tspackages/db/src/query/compiler/select.tspackages/db/src/query/effect.tspackages/db/src/query/index.tspackages/db/src/query/ir.tspackages/db/src/query/live/collection-config-builder.tspackages/db/src/query/live/collection-subscriber.tspackages/db/src/query/live/utils.tspackages/db/src/query/optimizer.tspackages/db/tests/query/builder/from.test.tspackages/db/tests/query/builder/union-all.test.tspackages/db/tests/query/case-when.test-d.tspackages/db/tests/query/case-when.test.tspackages/db/tests/query/compiler/subqueries.test.tspackages/db/tests/query/group-by.test.tspackages/db/tests/query/includes.test.tspackages/db/tests/query/union-all.test-d.tspackages/db/tests/query/union-all.test.ts
Comment on lines
+71
to
+81
| type CaseWhenResult< | ||
| TValues extends Array<CaseWhenValue>, | ||
| THasDefault extends boolean, | ||
| > = TValues[number] extends ExpressionLike | ||
| ? BasicExpression< | ||
| ExtractType<TValues[number]> | (THasDefault extends true ? never : null) | ||
| > | ||
| : CaseWhenWrapper< | ||
| | ExtractCaseWhenValue<TValues[number]> | ||
| | (THasDefault extends true ? never : undefined) | ||
| > |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "query/builder" | head -20Repository: TanStack/db
Length of output: 871
🏁 Script executed:
wc -l packages/db/src/query/builder/functions.tsRepository: TanStack/db
Length of output: 102
🏁 Script executed:
sed -n '1,100p' packages/db/src/query/builder/functions.ts | head -100Repository: TanStack/db
Length of output: 2531
🏁 Script executed:
sed -n '60,90p' packages/db/src/query/builder/functions.tsRepository: TanStack/db
Length of output: 890
🏁 Script executed:
sed -n '600,620p' packages/db/src/query/builder/functions.tsRepository: TanStack/db
Length of output: 573
🏁 Script executed:
sed -n '1,150p' packages/db/src/query/builder/functions.ts | tail -60Repository: TanStack/db
Length of output: 2493
🏁 Script executed:
grep -n "function caseWhen\|export.*caseWhen" packages/db/src/query/builder/functions.ts | head -20Repository: TanStack/db
Length of output: 589
🏁 Script executed:
sed -n '500,650p' packages/db/src/query/builder/functions.tsRepository: TanStack/db
Length of output: 3852
🏁 Script executed:
grep -n "caseWhenHasOnlyExpressionValues" packages/db/src/query/builder/functions.tsRepository: TanStack/db
Length of output: 190
🏁 Script executed:
sed -n '700,790p' packages/db/src/query/builder/functions.tsRepository: TanStack/db
Length of output: 2663
🏁 Script executed:
sed -n '44,58p' packages/db/src/query/builder/functions.tsRepository: TanStack/db
Length of output: 295
🏁 Script executed:
grep -n "type ExpressionLike" packages/db/src/query/builder/functions.tsRepository: TanStack/db
Length of output: 80
🏁 Script executed:
rg "QueryBuilder|ToArrayWrapper|ConcatToArrayWrapper" packages/db/src/query/builder/functions.ts -A 2 -B 2 | head -80Repository: TanStack/db
Length of output: 1506
🏁 Script executed:
cat > /tmp/test-distributivity.ts << 'EOF'
// Demonstrating the distributivity issue with CaseWhenResult type
type ExpressionLike = string | number | null | undefined
type QueryBuilder<T> = { __brand: 'QueryBuilder' }
// Original conditional (distributive)
type CaseWhenResult_Distributive<
TValues extends Array<ExpressionLike | QueryBuilder<any>>,
THasDefault extends boolean,
> = TValues[number] extends ExpressionLike
? { type: 'BasicExpression' }
: { type: 'CaseWhenWrapper' }
// Proposed fix (non-distributive)
type CaseWhenResult_NonDistributive<
TValues extends Array<ExpressionLike | QueryBuilder<any>>,
THasDefault extends boolean,
> = [Exclude<TValues[number], ExpressionLike>] extends [never]
? { type: 'BasicExpression' }
: { type: 'CaseWhenWrapper' }
// Test case 1: all ExpressionLike
type Test1_Dist = CaseWhenResult_Distributive<[string, number], false> // { type: 'BasicExpression' }
type Test1_NonDist = CaseWhenResult_NonDistributive<[string, number], false> // { type: 'BasicExpression' }
// Test case 2: mixed (string | QueryBuilder)
type Test2_Dist = CaseWhenResult_Distributive<[string, QueryBuilder<any>], false>
// Distributes: (string extends ExpressionLike ? BasicExpression : Wrapper) | (QueryBuilder extends ExpressionLike ? BasicExpression : Wrapper)
// = BasicExpression | CaseWhenWrapper ← TYPE LEAK: allows BasicExpression
type Test2_NonDist = CaseWhenResult_NonDistributive<[string, QueryBuilder<any>], false>
// Exclude<string | QueryBuilder, ExpressionLike> = QueryBuilder
// [QueryBuilder] extends [never] = false
// = CaseWhenWrapper ← CORRECT
const x: Test2_Dist = { type: 'BasicExpression' } // TypeScript allows this but runtime returns CaseWhenWrapper!
EOF
tsc /tmp/test-distributivity.ts --strict --noEmit 2>&1 || trueRepository: TanStack/db
Length of output: 197
🏁 Script executed:
fd "functions.test" packages/db/tests/query/builder/ -x cat {} \; | head -200Repository: TanStack/db
Length of output: 5760
🏁 Script executed:
fd "callback-types.test-d" packages/db/tests/ -x cat {} \; | grep -A 30 -B 5 "caseWhen" | head -100Repository: TanStack/db
Length of output: 37
🏁 Script executed:
fd ".*\.test-d\.ts$" packages/db/tests/query/builder/ -x cat {} \;Repository: TanStack/db
Length of output: 37195
🏁 Script executed:
rg "caseWhen" packages/db/tests/query/builder/ -A 5 -B 2Repository: TanStack/db
Length of output: 37
🏁 Script executed:
sed -n '732,780p' packages/db/src/query/builder/functions.tsRepository: TanStack/db
Length of output: 1440
🏁 Script executed:
sed -n '417,600p' packages/db/src/query/builder/functions.ts | grep -A 5 "CaseWhenResult"Repository: TanStack/db
Length of output: 1894
🏁 Script executed:
python3 << 'PYEOF'
# Verify the type logic conceptually
# ExpressionLike includes: Aggregate, BasicExpression, RefProxy, RefLeaf, string, number, boolean, bigint, Date, null, undefined
expression_like = {'Aggregate', 'BasicExpression', 'RefProxy', 'RefLeaf', 'string', 'number', 'boolean', 'bigint', 'Date', 'null', 'undefined'}
# CaseWhenValue adds: QueryBuilder, ToArrayWrapper, ConcatToArrayWrapper, Record
case_when_value = expression_like | {'QueryBuilder', 'ToArrayWrapper', 'ConcatToArrayWrapper', 'Record'}
print("ExpressionLike members:", expression_like)
print("\nCaseWhenValue additional members:", case_when_value - expression_like)
# Test case 1: all ExpressionLike (string, number)
test1_types = {'string', 'number'}
print(f"\nTest 1 - All ExpressionLike {test1_types}:")
print(f" All in ExpressionLike? {test1_types.issubset(expression_like)}")
print(f" Runtime returns: Func (BasicExpression)")
print(f" Current type: BasicExpression ✓")
# Test case 2: mixed (string, Record)
test2_types = {'string', 'Record'}
print(f"\nTest 2 - Mixed {test2_types}:")
print(f" All in ExpressionLike? {test2_types.issubset(expression_like)}")
print(f" With distributive conditional:")
print(f" string extends ExpressionLike = true → BasicExpression")
print(f" Record extends ExpressionLike = false → CaseWhenWrapper")
print(f" Result: BasicExpression | CaseWhenWrapper (TYPE LEAK!)")
print(f" Runtime returns: CaseWhenWrapper (because isExpressionValue(Record) = false)")
print(f" Problem: Type allows BasicExpression but runtime is CaseWhenWrapper")
# Test proposed fix with Exclude
print(f"\nProposed fix: [Exclude<TValues[number], ExpressionLike>] extends [never]")
print(f"\nTest 2 with fix - Mixed {test2_types}:")
remaining = test2_types - expression_like
print(f" Exclude<{test2_types}, ExpressionLike> = {remaining if remaining else 'never'}")
print(f" [{remaining if remaining else 'never'}] extends [never] = {remaining == set()}")
print(f" Result: CaseWhenWrapper ✓ (matches runtime)")
PYEOFRepository: TanStack/db
Length of output: 1162
Make the scalar-vs-wrapper split non-distributive.
TValues[number] extends ExpressionLike distributes over unions. A mixed call like caseWhen(cond1, 'x', cond2, { id: user.id }) is typed as BasicExpression | CaseWhenWrapper, but the runtime at line 609 only returns a scalar Func when all branch values are expression-like. Since Record is not in ExpressionLike, plain objects fail the isExpressionValue check and force CaseWhenWrapper at runtime, leaking select-only projections into where/orderBy/join type positions.
Suggested fix
type CaseWhenResult<
TValues extends Array<CaseWhenValue>,
THasDefault extends boolean,
-> = TValues[number] extends ExpressionLike
+> = [Exclude<TValues[number], ExpressionLike>] extends [never]
? BasicExpression<
ExtractType<TValues[number]> | (THasDefault extends true ? never : null)
>
: CaseWhenWrapper<
| ExtractCaseWhenValue<TValues[number]>
| (THasDefault extends true ? never : undefined)
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| type CaseWhenResult< | |
| TValues extends Array<CaseWhenValue>, | |
| THasDefault extends boolean, | |
| > = TValues[number] extends ExpressionLike | |
| ? BasicExpression< | |
| ExtractType<TValues[number]> | (THasDefault extends true ? never : null) | |
| > | |
| : CaseWhenWrapper< | |
| | ExtractCaseWhenValue<TValues[number]> | |
| | (THasDefault extends true ? never : undefined) | |
| > | |
| type CaseWhenResult< | |
| TValues extends Array<CaseWhenValue>, | |
| THasDefault extends boolean, | |
| > = [Exclude<TValues[number], ExpressionLike>] extends [never] | |
| ? BasicExpression< | |
| ExtractType<TValues[number]> | (THasDefault extends true ? never : null) | |
| > | |
| : CaseWhenWrapper< | |
| | ExtractCaseWhenValue<TValues[number]> | |
| | (THasDefault extends true ? never : undefined) | |
| > |
🤖 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/builder/functions.ts` around lines 71 - 81, The
conditional in the CaseWhenResult type is distributing over union members
causing mixed branch unions to be typed as a union of BasicExpression and
CaseWhenWrapper; change the distributive check to a non-distributive form so the
type only yields BasicExpression when all branch values are ExpressionLike.
Concretely, replace "TValues[number] extends ExpressionLike" with
"[TValues[number]] extends [ExpressionLike]" (and keep the THasDefault logic
intact) so CaseWhenResult correctly resolves to BasicExpression only when every
branch is expression-like and to CaseWhenWrapper otherwise; update the type
use-sites (e.g., where CaseWhenResult is returned from caseWhen) to rely on this
corrected behavior.
Comment on lines
+570
to
+596
| export function caseWhen< | ||
| C1 extends ExpressionLike, | ||
| V1 extends CaseWhenValue, | ||
| C2 extends ExpressionLike, | ||
| V2 extends CaseWhenValue, | ||
| C3 extends ExpressionLike, | ||
| V3 extends CaseWhenValue, | ||
| C4 extends ExpressionLike, | ||
| V4 extends CaseWhenValue, | ||
| C5 extends ExpressionLike, | ||
| V5 extends CaseWhenValue, | ||
| >( | ||
| condition1: C1, | ||
| value1: V1, | ||
| condition2: C2, | ||
| value2: V2, | ||
| condition3: C3, | ||
| value3: V3, | ||
| condition4: C4, | ||
| value4: V4, | ||
| condition5: C5, | ||
| value5: V5, | ||
| condition6: ExpressionLike, | ||
| value6: CaseWhenValue, | ||
| ...rest: Array<CaseWhenValue> | ||
| ): any | ||
| export function caseWhen(...args: Array<CaseWhenValue>): any { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/db/src/query/builder/functions.ts | sed -n '550,620p'Repository: TanStack/db
Length of output: 2225
🏁 Script executed:
rg -n "CaseWhenValue|ExpressionLike" packages/db/src/query/builder/functions.ts -B 2 -A 2 | head -100Repository: TanStack/db
Length of output: 2891
🏁 Script executed:
rg -n "export function caseWhen" packages/db/src/query/builder/functions.tsRepository: TanStack/db
Length of output: 505
🏁 Script executed:
cat -n packages/db/src/query/builder/functions.ts | sed -n '417,595p'Repository: TanStack/db
Length of output: 5450
The 6+ condition overload returns any, losing result type inference.
Lines 570-595 accept a 6th condition-value pair and rest arguments but return any instead of CaseWhenResult<...>. This breaks the typing pattern established for 1-5 pairs and leaks any into downstream query types. Add properly typed overloads for the 6-pair case (with and without default), or refactor to a variadic tuple signature that maintains type safety at all arities.
🤖 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/builder/functions.ts` around lines 570 - 596, The
caseWhen overload that accepts a 6th condition/value pair (the function
signature beginning with "export function caseWhen< C1...V5 >( ... condition6:
ExpressionLike, value6: CaseWhenValue, ...rest: Array<CaseWhenValue> ): any")
currently returns any and loses type information; update the overloads so they
return a properly typed CaseWhenResult (e.g.,
CaseWhenResult<C1,V1,C2,V2,C3,V3,C4,V4,C5,V5,C6,V6> or the appropriate generic
for the six-pair form), and add the matching overload for the six-pair plus
default value variant (or replace with a variadic-tuple generic that preserves
exact types for all arities). Ensure you reference the existing types
ExpressionLike, CaseWhenValue and CaseWhenResult and keep the runtime
implementation signature unchanged (only adjust the TypeScript overload
signatures) so downstream query types retain full type inference.
Comment on lines
+1126
to
+1129
| const childAliases: Array<string> = | ||
| childQuery.from.type === `unionFrom` | ||
| ? childQuery.from.sources.map((source) => source.alias) | ||
| : [childQuery.from.alias] |
Contributor
There was a problem hiding this comment.
Handle unionAll() child aliases when extracting include correlations.
For a child subquery built with query.unionAll(branchA, branchB), this fallback path uses childQuery.from.alias, so childAliases does not contain any of the branch aliases. extractCorrelation() then never matches refs like message.parentId/toolCall.parentId, and correlated includes fail with the “must have a WHERE eq()” error even when the query is valid. Please derive aliases from all unionAll branches here, not just unionFrom.
🤖 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/builder/index.ts` around lines 1126 - 1129, The
fallback that builds childAliases only handles childQuery.from.type ===
`unionFrom` and uses sources, but misses the `unionAll` case so branch aliases
(e.g., branchA/branchB) are not included and extractCorrelation never matches
those refs; update the logic around childAliases to also detect
`childQuery.from.type === 'unionAll'` and collect all branch aliases (e.g., map
over childQuery.from.branches or equivalent branch list to extract each
branch.alias) so extractCorrelation will see aliases from unionAll branches as
well.
Comment on lines
+590
to
+595
| type KeysOfUnion<T> = T extends unknown ? keyof T : never | ||
| type ValueOfUnion<T, K extends PropertyKey> = T extends unknown | ||
| ? K extends keyof T | ||
| ? T[K] | ||
| : never | ||
| : never |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "types.ts" | grep -E "query/builder"Repository: TanStack/db
Length of output: 96
🏁 Script executed:
if [ -f "packages/db/src/query/builder/types.ts" ]; then
echo "=== File exists. Getting line count ==="
wc -l packages/db/src/query/builder/types.ts
echo ""
echo "=== Lines 590-595 ==="
sed -n '590,595p' packages/db/src/query/builder/types.ts
fiRepository: TanStack/db
Length of output: 339
🏁 Script executed:
echo "=== Lines 580-610 for full context ==="
sed -n '580,610p' packages/db/src/query/builder/types.tsRepository: TanStack/db
Length of output: 1462
🏁 Script executed:
echo "=== Search for ValueOfUnion usage ==="
rg "ValueOfUnion" packages/db/src/query/builder/Repository: TanStack/db
Length of output: 1025
🏁 Script executed:
echo "=== Search for ContextFromUnionBranches and RefsForContext ==="
rg "ContextFromUnionBranches|RefsForContext" packages/db/src/query/builder/ -A 3Repository: TanStack/db
Length of output: 6264
🏁 Script executed:
echo "=== UnionBranchSchema definition ==="
sed -n '100,200p' packages/db/src/query/builder/types.ts | head -100Repository: TanStack/db
Length of output: 3486
🏁 Script executed:
echo "=== Search for UnionBranchSchema ==="
rg "UnionBranchSchema" packages/db/src/query/builder/ -B 2 -A 8Repository: TanStack/db
Length of output: 1810
🏁 Script executed:
echo "=== Full context for RefsForContext (lines 600-650) ==="
sed -n '600,650p' packages/db/src/query/builder/types.tsRepository: TanStack/db
Length of output: 2371
🏁 Script executed:
echo "=== Looking at how RefsForContext uses ValueOfUnion ==="
sed -n '600,640p' packages/db/src/query/builder/types.tsRepository: TanStack/db
Length of output: 1853
🏁 Script executed:
echo "=== Looking for test cases or examples with unionAll ==="
rg "unionAll" packages/db/src --type ts -B 2 -A 5 | head -80Repository: TanStack/db
Length of output: 5102
🏁 Script executed:
echo "=== Check how RefsForContext works with union contexts ==="
rg "hasUnionFrom|refsSchema" packages/db/src/query/builder/types.ts -B 1 -A 1Repository: TanStack/db
Length of output: 1748
🏁 Script executed:
echo "=== Check the current state - is ValueOfUnion still returning never? ==="
sed -n '590,595p' packages/db/src/query/builder/types.tsRepository: TanStack/db
Length of output: 305
🏁 Script executed:
echo "=== Verify the problem: check if refs are correctly marked as optional for union branches ==="
echo "Example: For unionAll of { message: Message } | { toolCall: ToolCall }:"
echo "Both keys should appear in RefsForContext, but message and toolCall should be optional"
echo ""
echo "Let's check if there's any test or usage showing this works correctly:"
rg "unionAll" packages/db -A 20 | grep -E "(where|orderBy|select)" | head -20Repository: TanStack/db
Length of output: 2181
🏁 Script executed:
echo "=== Check if the issue is acknowledged in comments ==="
rg -i "union.*optional|optional.*union|branch.*undefined" packages/db/src/query/builder/types.tsRepository: TanStack/db
Length of output: 240
🏁 Script executed:
echo "=== Examine the union test file to see if refs are being tested correctly ==="
head -100 packages/db/tests/query/union-all.test-d.tsRepository: TanStack/db
Length of output: 2438
🏁 Script executed:
echo "=== Look at the actual test case with message and toolCall ==="
rg "message.*toolCall|toolCall.*message" packages/db/tests/query/union-all.test-d.ts -B 5 -A 10 | head -50Repository: TanStack/db
Length of output: 1439
🏁 Script executed:
echo "=== Check RefsForContext logic for optional detection ==="
sed -n '605,620p' packages/db/src/query/builder/types.tsRepository: TanStack/db
Length of output: 892
🏁 Script executed:
echo "=== Look for IsNonExactOptional to see how it detects optionality ==="
rg "IsNonExactOptional" packages/db/src/query/builder/types.ts -B 2 -A 2Repository: TanStack/db
Length of output: 971
🏁 Script executed:
echo "=== Verify the type issue: trace UnionBranchSchema behavior ==="
echo "Looking at what UnionBranchSchema produces for a union type:"
sed -n '155,165p' packages/db/src/query/builder/types.tsRepository: TanStack/db
Length of output: 615
🏁 Script executed:
echo "=== Check the exact semantics: verify if the schema values are being made optional ==="
rg "UnionRefsSchema" packages/db/src/query/builder/types.ts -B 2 -A 5Repository: TanStack/db
Length of output: 617
🏁 Script executed:
echo "=== Trace through the problem: what type does UnionBranchSchema actually produce? ==="
echo "For union { message: Message } | { toolCall: ToolCall }:"
echo "KeysOfUnion produces: 'message' | 'toolCall'"
echo "Current ValueOfUnion for 'toolCall' in first branch: never"
echo "Union of never with ToolCall: ToolCall"
echo "So schema becomes: { message: Message, toolCall: ToolCall } - BOTH REQUIRED"
echo ""
echo "If we change to undefined:"
echo "Current ValueOfUnion for 'toolCall' in first branch: undefined"
echo "Union of undefined with ToolCall: ToolCall | undefined"
echo "So schema becomes: { message: Message | undefined, toolCall: ToolCall | undefined } - BOTH OPTIONAL"
echo ""
echo "Let's verify the refsSchema field gets passed correctly:"
sed -n '593,610p' packages/db/src/query/builder/types.tsRepository: TanStack/db
Length of output: 1448
🏁 Script executed:
echo "=== Final verification: test if current code requires guards or not ==="
echo "Looking for whether the test needs coalesce because refs aren't properly optional:"
sed -n '100,160p' packages/db/tests/query/union-all.test-d.tsRepository: TanStack/db
Length of output: 2005
🏁 Script executed:
echo "=== Check if select() test shows the issue more clearly ==="
sed -n '120,180p' packages/db/tests/query/union-all.test-d.tsRepository: TanStack/db
Length of output: 2124
🏁 Script executed:
echo "=== Critical check: does accessing .text on message actually cause a type error? ==="
echo "Let's look at whether the test is actually testing what we think it is testing"
echo "The test accesses message.text and toolCall.name without guards."
echo "If RefsForContext didn't mark these as optional, it would be a type error."
echo ""
echo "Let's check if there's a test for the error case:"
rg "`@ts-expect-error`" packages/db/tests/query/union-all.test-d.tsRepository: TanStack/db
Length of output: 561
🏁 Script executed:
echo "=== Simulate: what happens when you access property on undefined ref? ==="
echo "Let's check if refs properly allow undefined access or if there's compensation:"
sed -n '140,165p' packages/db/tests/query/union-all.test-d.tsRepository: TanStack/db
Length of output: 1202
🏁 Script executed:
echo "=== Check the Ref type to see if it allows undefined property access ==="
rg "export type Ref<" packages/db/src/query/builder/types.ts -A 30 | head -50Repository: TanStack/db
Length of output: 1365
🏁 Script executed:
echo "=== The critical question: Is the test actually correct? ==="
echo "Let's check if accessing message.text without a guard should be a type error:"
echo ""
echo "For a union { message: MessageRow } | { toolCall: ToolCallRow },"
echo "a ref for 'message' should be optional (could be undefined)."
echo "So accessing .text on it requires a guard or coalesce."
echo ""
echo "The test uses coalesce() for timestamp but accesses .text directly."
echo "This suggests the types are not properly marked as optional."
echo ""
echo "Let's verify: check if select callback has stricter checking:"
rg "select.*message.*toolCall" packages/db/tests/query/union-all.test-d.ts -B 3 -A 8Repository: TanStack/db
Length of output: 959
🏁 Script executed:
echo "=== Let's check the actual branch logic - is coalesce hiding the type issue? ==="
echo "Looking at what coalesce does and why it's used for timestamp but not text:"
sed -n '145,160p' packages/db/tests/query/union-all.test-d.tsRepository: TanStack/db
Length of output: 869
🏁 Script executed:
echo "=== Verify the actual problem by checking if there's a simpler test ==="
rg "message\..*|toolCall\." packages/db/tests/query/union-all.test-d.tsRepository: TanStack/db
Length of output: 1251
🏁 Script executed:
echo "=== Final definitive check: trace the exact schema being used ==="
echo "For unionAll with branches returning { message: Message } | { toolCall: ToolCall }:"
echo ""
echo "1. UnionBranchResult extracts the union result type"
echo "2. UnionBranchSchema maps it:"
sed -n '145,152p' packages/db/src/query/builder/types.ts
echo ""
echo "3. ContextFromUnionBranches sets refsSchema to UnionBranchSchema:"
sed -n '171,177p' packages/db/src/query/builder/types.ts
echo ""
echo "4. RefsSchemaForContext chooses refsSchema if available:"
sed -n '596,605p' packages/db/src/query/builder/types.tsRepository: TanStack/db
Length of output: 1481
🏁 Script executed:
echo "=== The key insight: trace what UnionBranchSchema actually produces ==="
echo "Test the type manually by checking what happens with never in unions:"
python3 << 'EOF'
# Simulate what happens with union types
print("For union { message: Message } | { toolCall: ToolCall }:")
print("")
print("KeysOfUnion produces: 'message' | 'toolCall'")
print("")
print("UnionBranchSchema maps each key:")
print(" For 'message':")
print(" Branch 1: { message: Message } → ValueOfUnion(..., 'message') = Message")
print(" Branch 2: { toolCall: ToolCall } → ValueOfUnion(..., 'message') = never")
print(" Result: Message | never = Message")
print("")
print(" For 'toolCall':")
print(" Branch 1: { message: Message } → ValueOfUnion(..., 'toolCall') = never")
print(" Branch 2: { toolCall: ToolCall } → ValueOfUnion(..., 'toolCall') = ToolCall")
print(" Result: never | ToolCall = ToolCall ← PROBLEM! Should be ToolCall | undefined")
print("")
print("Final schema: { message: Message, toolCall: ToolCall }")
print("Expected: { message: Message | undefined, toolCall: ToolCall | undefined }")
EOFRepository: TanStack/db
Length of output: 929
Preserve undefined for keys missing in union branches.
ValueOfUnion returns never for a key that doesn't exist in a union branch. When unionizing results like { message: Message } | { toolCall: ToolCall }, this causes ValueOfUnion to produce never | ToolCall = ToolCall for the toolCall key, making it appear required even though only one branch contains it. Changing never to undefined makes the union undefined | ToolCall, correctly representing optionality. This ensures RefsForContext properly detects branch-exclusive fields as optional.
Suggested fix
type ValueOfUnion<T, K extends PropertyKey> = T extends unknown
? K extends keyof T
? T[K]
- : never
+ : undefined
: never📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| type KeysOfUnion<T> = T extends unknown ? keyof T : never | |
| type ValueOfUnion<T, K extends PropertyKey> = T extends unknown | |
| ? K extends keyof T | |
| ? T[K] | |
| : never | |
| : never | |
| type KeysOfUnion<T> = T extends unknown ? keyof T : never | |
| type ValueOfUnion<T, K extends PropertyKey> = T extends unknown | |
| ? K extends keyof T | |
| ? T[K] | |
| : undefined | |
| : never |
🤖 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/builder/types.ts` around lines 590 - 595, The
ValueOfUnion helper currently returns never when a union branch lacks key K
which makes branch-exclusive fields appear required; update ValueOfUnion (the
conditional type) so that when K is not a key of a branch it yields undefined
instead of never, preserving undefined for missing keys and making fields like
toolCall optional in unions used by RefsForContext; locate the ValueOfUnion type
in types.ts and change the branch that now produces never to produce undefined
while keeping the existing conditional structure.
Comment on lines
+787
to
+850
| function replaceGroupByRefsInSelectValue( | ||
| value: SelectValueExpression, | ||
| groupByClause: GroupBy, | ||
| ): SelectValueExpression { | ||
| if (isConditionalSelect(value)) { | ||
| return new ConditionalSelect( | ||
| value.branches.map((branch) => ({ | ||
| condition: replaceGroupByRefsInExpression( | ||
| branch.condition, | ||
| groupByClause, | ||
| ), | ||
| value: replaceGroupByRefsInSelectValue(branch.value, groupByClause), | ||
| })), | ||
| value.defaultValue === undefined | ||
| ? undefined | ||
| : replaceGroupByRefsInSelectValue(value.defaultValue, groupByClause), | ||
| ) | ||
| } | ||
|
|
||
| if (isNestedSelectObject(value)) { | ||
| const transformed: Select = {} | ||
| for (const [key, entry] of Object.entries(value)) { | ||
| transformed[key] = replaceGroupByRefsInSelectValue( | ||
| entry as SelectValueExpression, | ||
| groupByClause, | ||
| ) | ||
| } | ||
| return transformed | ||
| } | ||
|
|
||
| if (!isExpressionLike(value)) { | ||
| return value | ||
| } | ||
|
|
||
| if (value.type === `includesSubquery` || value.type === `agg`) { | ||
| return value | ||
| } | ||
|
|
||
| return replaceGroupByRefsInExpression(value, groupByClause) | ||
| } | ||
|
|
||
| function replaceGroupByRefsInExpression( | ||
| expr: BasicExpression, | ||
| groupByClause: GroupBy, | ||
| ): BasicExpression { | ||
| if (expr.type === `ref`) { | ||
| const groupIndex = groupByClause.findIndex((groupExpr) => | ||
| expressionsEqual(expr, groupExpr), | ||
| ) | ||
| return groupIndex === -1 | ||
| ? expr | ||
| : new PropRef([`$selected`, `${GROUP_KEY_REF_PREFIX}${groupIndex}`]) | ||
| } | ||
|
|
||
| if (expr.type === `func`) { | ||
| return new Func( | ||
| expr.name, | ||
| expr.args.map((arg) => | ||
| replaceGroupByRefsInExpression(arg, groupByClause), | ||
| ), | ||
| ) | ||
| } | ||
|
|
||
| return expr |
Contributor
There was a problem hiding this comment.
Rewrite computed GROUP BY expressions, not just bare refs.
This only substitutes direct refs. If the group key is computed, e.g. groupBy(lower(user.name)), a wrapped projection like caseWhen(..., lower(user.name)) still runs against { $selected: ... } and resolves user.name as undefined instead of using the group key.
🐛 Proposed fix
function replaceGroupByRefsInExpression(
expr: BasicExpression,
groupByClause: GroupBy,
): BasicExpression {
+ const groupIndex = groupByClause.findIndex((groupExpr) =>
+ expressionsEqual(expr, groupExpr),
+ )
+ if (groupIndex !== -1) {
+ return new PropRef([`$selected`, `${GROUP_KEY_REF_PREFIX}${groupIndex}`])
+ }
+
if (expr.type === `ref`) {
- const groupIndex = groupByClause.findIndex((groupExpr) =>
- expressionsEqual(expr, groupExpr),
- )
- return groupIndex === -1
- ? expr
- : new PropRef([`$selected`, `${GROUP_KEY_REF_PREFIX}${groupIndex}`])
+ return expr
}
if (expr.type === `func`) {
Comment on lines
440
to
446
| if (query.select) { | ||
| const includesEntries = extractIncludesFromSelect(query.select) | ||
| // Shallow-clone select before mutating so we don't modify the shared IR | ||
| // (the optimizer copies select by reference, so rawQuery.select === query.select) | ||
| if (includesEntries.length > 0) { | ||
| query = { ...query, select: { ...query.select } } | ||
| } |
Contributor
There was a problem hiding this comment.
Recursive include replacement still mutates the shared query tree.
Only the top-level select object is cloned here, but replaceIncludesInSelect() now descends into ConditionalSelect branches and nested objects and mutates them in place. The first compile of a guarded/nested include will rewrite the original/raw IR to ValClass(null), so later compiles/subscriptions silently lose that include. Make the replacement path-copying or return a transformed tree instead of mutating nested nodes.
Also applies to: 1878-1933
🤖 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 440 - 446, The current
clone only shallow-copies query.select but replaceIncludesInSelect() mutates
nested branches (including ConditionalSelect), which modifies the shared IR;
change the flow so replacements are done immutably: update or replace
replaceIncludesInSelect to return a new transformed Select tree (or perform
path-copying on any node it touches) instead of mutating nodes in place, and
invoke it to assign the returned select to query.select; also apply the same
immutable transformation approach to the other occurrence referenced around the
1878-1933 region to ensure no nested objects are mutated.
Comment on lines
+122
to
+166
| function getTargetsFromPropRef( | ||
| query: QueryIR, | ||
| ref: PropRef, | ||
| ): Array<LazyLoadTarget> { | ||
| if (ref.path.length === 0) { | ||
| return [] | ||
| } | ||
|
|
||
| if (ref.path.length === 1) { | ||
| const field = ref.path[0]! | ||
| const selectedField = query.select?.[field] | ||
| if (selectedField) { | ||
| return getTargetsFromExpression(query, selectedField) | ||
| } | ||
| return [] | ||
| } | ||
|
|
||
| const [alias, ...path] = ref.path | ||
| const source = getSourceFromAlias(query.from, alias!) | ||
| if (!source) { | ||
| return [] | ||
| } | ||
|
|
||
| if (source.type === `collectionRef`) { | ||
| return [{ alias: source.alias, collection: source.collection, path }] | ||
| } | ||
|
|
||
| if (source.query.limit || source.query.offset) { | ||
| return [] | ||
| } | ||
|
|
||
| return getTargetsFromQueryRef(source.query, source.alias, ref) | ||
| } | ||
|
|
||
| function getSourceFromAlias( | ||
| from: From, | ||
| alias: string, | ||
| ): CollectionRef | QueryRef | undefined { | ||
| const sources = | ||
| from.type === `unionFrom` | ||
| ? from.sources | ||
| : from.type === `unionAll` | ||
| ? [] | ||
| : [from] | ||
| return sources.find((source) => source.alias === alias) |
Contributor
There was a problem hiding this comment.
Join-backed projections can't resolve to lazy targets.
getSourceFromAlias() only checks from, so a projection like select({ parentId: joinedAlias.id }) inside a union/subquery branch returns no target here even though the alias is valid on query.join. That silently disables targeted snapshot loading for joined branches/subqueries and falls back to full collection loads.
🤖 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/lazy-targets.ts` around lines 122 - 166,
getSourceFromAlias currently only looks at the top-level from and misses aliases
defined on query joins or inside queryRef/union branches, causing
getTargetsFromPropRef to miss valid join-backed aliases; update
getSourceFromAlias (and usages in getTargetsFromPropRef/getTargetsFromQueryRef)
to collect and search aliases from: the top-level from sources, any nested
QueryRef sources' query.from and query.join arrays (and for unionFrom/unionAll
iterate each branch's from and its query joins), so that when a matching alias
exists in a join or inside a queryRef branch you return the correct
CollectionRef or QueryRef instead of undefined. Ensure you still respect
existing branch logic (e.g., unionAll -> []) and preserve source.type checks
like collectionRef vs queryRef.
Comment on lines
+118
to
+121
| } else if (from.type === `unionFrom`) { | ||
| return extractCollectionFromSource({ from: from.sources[0] }) | ||
| } else if (from.type === `unionAll`) { | ||
| return extractCollectionFromSource(from.queries[0]) |
Contributor
There was a problem hiding this comment.
Don't inherit collation from an arbitrary union branch.
extractCollectionFromSource() now resolves unionFrom/unionAll by taking the first branch, but CollectionConfigBuilder uses this helper to inherit defaultStringCollation when callers do not provide one. For multi-source queries, that makes string comparison/order semantics depend on branch order whenever the sources have different compareOptions. Please either validate that all union branches agree on collation or require an explicit defaultStringCollation for union queries.
🤖 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/live/utils.ts` around lines 118 - 121,
extractCollectionFromSource currently returns the first branch for
unionFrom/unionAll which lets CollectionConfigBuilder implicitly inherit
defaultStringCollation from an arbitrary branch; instead, update
extractCollectionFromSource (or CollectionConfigBuilder's use of it) to either
validate that all branches in unionFrom/unionAll share the same
compareOptions/defaultStringCollation and throw if they differ, or require
callers to pass an explicit defaultStringCollation for union queries;
specifically, when encountering from.type === 'unionFrom' or 'unionAll', iterate
all sources/queries and compare their compareOptions/defaultStringCollation
values, and if any mismatch is detected raise an error (or clear the inherited
collation and force an explicit parameter) so collation is not silently taken
from the first branch.
Comment on lines
+908
to
+909
| function getFirstFromAlias(query: QueryIR): string { | ||
| return getFromSources(query.from)[0]!.alias |
Contributor
There was a problem hiding this comment.
Guard unionAll before dereferencing the first source alias.
getFromSources() returns [] for unionAll, so getFirstFromAlias() can throw here when referencesAliasWithRemappedSelect() inspects a projected PropRef on a unionAll-backed subquery. That turns a valid joined union query into an optimizer crash instead of a conservative “don’t push down” decision.
💡 Suggested fix
-function getFirstFromAlias(query: QueryIR): string {
- return getFromSources(query.from)[0]!.alias
+function getFirstFromAlias(query: QueryIR): string | undefined {
+ return getFromSources(query.from)[0]?.alias
}
...
+ const firstFromAlias = getFirstFromAlias(subquery)
if (
innerAlias !== outerAlias &&
- innerAlias !== getFirstFromAlias(subquery)
+ innerAlias !== firstFromAlias
) {
return true
}Also applies to: 1194-1197
🤖 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/optimizer.ts` around lines 908 - 909, getFirstFromAlias
currently dereferences getFromSources(...)[0] which throws for unionAll (no
sources); update getFirstFromAlias to guard for an empty array (or query.kind
=== 'unionAll') and return a safe sentinel (e.g. empty string) instead of
indexing into [0]; also apply the same guard to the similar code paths around
the referencesAliasWithRemappedSelect call (the occurrence at lines ~1194-1197)
so callers receive a safe value and the optimizer will conservatively avoid
pushdown instead of crashing.
Comment on lines
+127
to
+324
| test(`infers conditional projection values`, () => { | ||
| const users = createUsers() | ||
| const query = createLiveQueryCollection((q) => | ||
| q.from({ user: users }).select(({ user }) => { | ||
| const adultProfile = caseWhen(gt(user.age, 18), { | ||
| id: user.id, | ||
| name: user.name, | ||
| }) | ||
| return { | ||
| id: user.id, | ||
| adultProfile, | ||
| } | ||
| }), | ||
| ) | ||
|
|
||
| const result = query.toArray[0]! | ||
|
|
||
| expectTypeOf(result).toExtend< | ||
| OutputWithVirtualKeyed<{ | ||
| id: number | ||
| adultProfile: | ||
| | { | ||
| id: number | ||
| name: string | ||
| } | ||
| | undefined | ||
| }> | ||
| >() | ||
| }) | ||
|
|
||
| test(`infers includes inside conditional projection values`, () => { | ||
| const users = createUsers() | ||
| const posts = createPosts() | ||
| const query = createLiveQueryCollection((q) => | ||
| q.from({ user: users }).select(({ user }) => ({ | ||
| id: user.id, | ||
| adultProfile: caseWhen(gt(user.age, 18), { | ||
| id: user.id, | ||
| postTitles: toArray( | ||
| q | ||
| .from({ post: posts }) | ||
| .where(({ post }) => eq(post.userId, user.id)) | ||
| .select(({ post }) => post.title), | ||
| ), | ||
| }), | ||
| })), | ||
| ) | ||
|
|
||
| const result = query.toArray[0]! | ||
|
|
||
| expectTypeOf(result).toExtend< | ||
| OutputWithVirtualKeyed<{ | ||
| id: number | ||
| adultProfile: | ||
| | { | ||
| id: number | ||
| postTitles: Array<string> | ||
| } | ||
| | undefined | ||
| }> | ||
| >() | ||
| }) | ||
|
|
||
| test(`infers Collection includes inside conditional projection values`, () => { | ||
| const users = createUsers() | ||
| const posts = createPosts() | ||
| const query = createLiveQueryCollection((q) => | ||
| q.from({ user: users }).select(({ user }) => ({ | ||
| id: user.id, | ||
| adultProfile: caseWhen(gt(user.age, 18), { | ||
| id: user.id, | ||
| posts: q | ||
| .from({ post: posts }) | ||
| .where(({ post }) => eq(post.userId, user.id)) | ||
| .select(({ post }) => ({ | ||
| title: post.title, | ||
| })), | ||
| }), | ||
| })), | ||
| ) | ||
|
|
||
| const result = query.toArray[0]! | ||
|
|
||
| expectTypeOf(result).toExtend< | ||
| OutputWithVirtualKeyed<{ | ||
| id: number | ||
| adultProfile: | ||
| | { | ||
| id: number | ||
| posts: { | ||
| toArray: Array<OutputWithVirtual<{ title: string }>> | ||
| } | ||
| } | ||
| | undefined | ||
| }> | ||
| >() | ||
| }) | ||
|
|
||
| test(`infers projection variadic branch values`, () => { | ||
| const users = createUsers() | ||
| const query = createLiveQueryCollection((q) => | ||
| q.from({ user: users }).select(({ user }) => ({ | ||
| id: user.id, | ||
| profile: caseWhen( | ||
| gt(user.age, 25), | ||
| { | ||
| kind: `senior`, | ||
| id: user.id, | ||
| }, | ||
| gt(user.age, 18), | ||
| { | ||
| kind: `adult`, | ||
| name: user.name, | ||
| }, | ||
| { | ||
| kind: `minor`, | ||
| active: user.active, | ||
| }, | ||
| ), | ||
| maybeProfile: caseWhen( | ||
| gt(user.age, 25), | ||
| { | ||
| kind: `senior`, | ||
| id: user.id, | ||
| }, | ||
| gt(user.age, 18), | ||
| { | ||
| kind: `adult`, | ||
| name: user.name, | ||
| }, | ||
| ), | ||
| })), | ||
| ) | ||
|
|
||
| const result = query.toArray[0]! | ||
|
|
||
| expectTypeOf(result).toExtend< | ||
| OutputWithVirtualKeyed<{ | ||
| id: number | ||
| profile: | ||
| | { | ||
| kind: string | ||
| id: number | ||
| } | ||
| | { | ||
| kind: string | ||
| name: string | ||
| } | ||
| | { | ||
| kind: string | ||
| active: boolean | ||
| } | ||
| maybeProfile: | ||
| | { | ||
| kind: string | ||
| id: number | ||
| } | ||
| | { | ||
| kind: string | ||
| name: string | ||
| } | ||
| | undefined | ||
| }> | ||
| >() | ||
| }) | ||
|
|
||
| test(`accepts source alias conditions`, () => { | ||
| const users = createUsers() | ||
| const posts = createPosts() | ||
| const query = createLiveQueryCollection((q) => | ||
| q | ||
| .from({ user: users }) | ||
| .join( | ||
| { post: posts }, | ||
| ({ user, post }) => eq(user.id, post.userId), | ||
| `left`, | ||
| ) | ||
| .select(({ user, post }) => ({ | ||
| id: user.id, | ||
| postStatus: caseWhen(post, `has-post`, `no-post`), | ||
| postProfile: caseWhen(post, { | ||
| hasPost: true, | ||
| }), | ||
| })), | ||
| ) | ||
|
|
||
| const result = query.toArray[0]! | ||
|
|
||
| expectTypeOf(result).toExtend< | ||
| OutputWithVirtualKeyed<{ | ||
| id: number | ||
| postStatus: string | ||
| postProfile: | ||
| | { | ||
| hasPost: boolean | ||
| } | ||
| | undefined | ||
| }> |
Contributor
There was a problem hiding this comment.
Resolve undefined vs null contract drift for conditional projection branches.
Type assertions here model non-matching projection branches as undefined (for example, Line 152/185/220/288/323), but runtime expectations in packages/db/tests/query/case-when.test.ts model equivalent branches as null (for example, Line 213/307/461). Please align runtime behavior, type tests, and docs to one representation to avoid unsound consumer expectations.
🤖 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/case-when.test-d.ts` around lines 127 - 324, Tests
and types disagree on whether non-matching caseWhen branches are represented as
undefined or null; fix by making the representation consistent. Update the
caseWhen implementation and its type signature (the function named caseWhen) to
return null for non-matching branches (or, if you prefer the opposite, update
the runtime to return undefined) and then align all type tests (references:
adultProfile, postProfile, profile, maybeProfile, and any caseWhen usages in
these tests) to expect that same nullable value (replace | undefined with | null
across the failing expectTypeOf assertions) so runtime, types, and docs all
match.
Contributor
There was a problem hiding this comment.
@wesleymatosinfluencer — it looks like your reply might have been intended for a different PR or review thread. The fix you describe (pulling now() into a locked SELECT to address a clock skew bug in a rate limiter) doesn't appear to be related to this PR or the concern I raised.
My original comment here was about an inconsistency in packages/db/tests/query/case-when.test-d.ts: the type-level tests assert that non-matching caseWhen projection branches produce undefined (e.g. | undefined on lines ~152, 185, 220, 288, 323), while the corresponding runtime tests in packages/db/tests/query/case-when.test.ts observe null for the same branches (e.g. lines ~213, 307, 461). These two representations need to be aligned so consumer type expectations match actual runtime values.
Could you confirm whether you meant to post that reply elsewhere? If so, the original undefined/null drift concern is still open on this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack
This PR is stacked on #1536 (
caseWhen) and usescase-whenas its base branch. Please review/merge #1536 first.Implementation plan: https://gist.github.com/samwillis/df7e2376c6c2bfd007e53dec797fca12
Summary
This PR adds multi-source
fromsupport to the DB query engine. It lets a query union multiple independent sources into one live query result without requiring an explicit join between those sources.What Changed
UnionFromIR node forfrom({ a, b, ... })with multiple sources.fromaccepts multiple collection or subquery sources, while joins still accept one source.wheresemantics after the union, while allowing users to pre-filter a branch by making it a subquery.from, including branch-dependent join keys viacoalesce.fromand ordering behavior in the live queries guide.Examples
Basic multi-source query:
Without
select, the result is an exclusive union:Pre-filter one branch with a subquery:
Join after a multi-source
fromwith branch-dependent keys:Guard branch-specific projections and includes with
caseWhenfrom #1536:Test Plan
pnpm vitest run packages/db/tests/query/multi-source-from.test.tspnpm --filter @tanstack/db buildpnpm vitest --run --typecheck tests/query/multi-source-from.test-d.ts --coverage.enabled=falsefrompackages/db; Vitest still reports unrelated existingincludes.test.tstype errors in the wider typecheck pass.Made with Cursor
Summary by CodeRabbit
Release Notes
New Features
caseWhenoperator for conditional query expressions and conditional projections with support for multi-branch conditions, default values, and use inwhere,select,groupBy, and other query contexts.unionAll()method to combine multiple data sources or query branches with unified result typing, ordering, and filtering capabilities across branches.Documentation
unionAllusage patterns and behavior across joins and projections.Made with Cursor