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 introduces the ChangescaseWhen Conditional Expression Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
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.86 kB (+2.5%) Total Size: 117 kB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
|
Size Change: 0 B Total Size: 4.24 kB ℹ️ View Unchanged
|
![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: 2
🧹 Nitpick comments (5)
packages/db/tests/query/case-when.test-d.ts (1)
31-49: ⚡ Quick winAdd explicit return types to test helper factories.
Line 31 and Line 41 rely on inferred return types; please annotate both function return types explicitly to keep type tests stable against inference drift.
Proposed diff
-function createUsers() { +function createUsers(): ReturnType<typeof createCollection<User>> { return createCollection( mockSyncCollectionOptions<User>({ id: `case-when-type-users`, getKey: (user) => user.id, initialData: [], }), ) } -function createPosts() { +function createPosts(): ReturnType<typeof createCollection<Post>> { return createCollection( mockSyncCollectionOptions<Post>({ id: `case-when-type-posts`, getKey: (post) => post.id, initialData: [], }), ) }As per coding guidelines, "
**/*.{ts,tsx}: Provide proper type annotations for function return values instead of relying on implicit types" and "Provide precise return types for functions; avoidunknownoranyreturn types unless absolutely necessary".🤖 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 helper factory functions createUsers and createPosts rely on inferred return types; add explicit return type annotations to both (for example, the concrete collection type returned by createCollection for User and Post or ReturnType<typeof createCollection> specialized for each) so the test helpers have stable, precise return types; update the function signatures for createUsers and createPosts to include those explicit return types.packages/db/tests/query/case-when.test.ts (2)
384-583: ⚡ Quick winAdd an explicit empty-include corner-case assertion.
The conditional include materialization tests are strong, but they don’t explicitly assert behavior when the included child query resolves to an empty array (
[]) in an active branch.As per coding guidelines, "**/*.test.{ts,tsx,js,jsx}: 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 384 - 583, Add an assertion that explicitly covers the empty-include corner case by ensuring a branch that should include child rows returns an empty array when there are no matching child rows: modify one of the conditional projection tests (e.g., the tests using createUsersCollection/createPostsCollection and the LiveQuery named query) to either include a user with no posts or adapt an existing user to have no posts, call query.preload(), then assert that the included field (postTitles or profile.posts) for that user is exactly [] using query.toArray (optionally wrapped by stripVirtualPropsAndSymbols or childRows for Collection includes) so the test verifies an empty-array include is handled correctly.
59-77: ⚡ Quick winReplace
anyin test helpers with safer types.Both helpers use
anyfor parameters and return types, which removes compile-time type checking even in tests.Suggested typed rewrite
-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> = {} - for (const [key, entry] of Object.entries(stripVirtualProps(value))) { + const out: Record<string, unknown> = {} + for (const [key, entry] of Object.entries(stripVirtualProps(value as object))) { 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: Iterable<unknown> }, +): Array<unknown> { + return [...collection.toArray].map((row) => stripVirtualPropsAndSymbols(row)) }Per coding guidelines: "**/*.{ts,tsx}: 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/tests/query/case-when.test.ts` around lines 59 - 77, Replace the unsafe any usages in the test helpers by typing stripVirtualPropsAndSymbols(value: unknown): unknown (and arrays as unknown[]) and childRows(collection: { toArray: Iterable<unknown> }): unknown[]; keep the existing runtime checks (Array.isArray, typeof value === 'object' && value !== null) as the type guards, change the intermediate out to Record<string, unknown>, and treat entries/returned values from stripVirtualProps as unknown before recursing so you avoid any while preserving the current logic in stripVirtualPropsAndSymbols, childRows, and the use of collection.toArray.packages/db/src/query/compiler/select.ts (1)
204-245: ⚡ Quick winExtract
isCaseWhenConditionTrue()into a shared helper.This truthiness rule now exists here and again in
compiler/evaluators.ts. If one side changes, scalarcaseWhen()and projectioncaseWhen()will pick different branches for the same row.As per coding guidelines, "Extract common logic into reusable utility functions when duplicated across multiple places".
🤖 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/select.ts` around lines 204 - 245, The truthiness logic in isCaseWhenConditionTrue is duplicated; extract it into a single shared helper (e.g., export function isCaseWhenConditionTrue from a new/shared utility module) and update compileConditionalSelect to import and use that helper instead of its local implementation, then update the other occurrence in compiler/evaluators.ts (and any caseWhen implementations) to import the same helper so scalar and projection caseWhen use identical logic; ensure the helper is exported with the same name and update imports where isCaseWhenConditionTrue was previously defined or referenced.packages/db/src/query/compiler/group-by.ts (1)
656-666: ⚡ Quick winUse proper types from the IR module.
The inline type assertion doesn't reference the actual
ConditionalSelecttype from the IR module. This creates a maintenance risk if the structure changes.♻️ Refactor to use imported types
Import
ConditionalSelectfrom the IR module (if not already imported), then refine the type check:if (expr.type === `conditionalSelect` && `branches` in expr) { + const conditionalExpr = expr as ConditionalSelect return ( - expr.branches as Array<{ - condition: BasicExpression - value: BasicExpression | Aggregate | Select | { type: string } - }> + conditionalExpr.branches ).some( (branch) => containsAggregate(branch.condition) || containsAggregate(branch.value), ) }Note: Ensure
ConditionalSelectis imported at the top of the file alongside other IR types.As per coding guidelines, avoid using
anytypes and provide proper type annotations instead of relying on type assertions.🤖 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/group-by.ts` around lines 656 - 666, Replace the inline ad-hoc type assertion for conditional selects with the actual ConditionalSelect IR type: import ConditionalSelect from the IR module (or add it to the existing IR import), then narrow expr using that type (e.g., treat expr as ConditionalSelect after the type check `expr.type === 'conditionalSelect'`) and access expr.branches typed as ConditionalSelect['branches'] instead of an anonymous cast; update the import section at the top of group-by.ts accordingly and adjust the containsAggregate checks to operate on the properly typed branch elements.
🤖 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/index.ts`:
- Around line 901-905: buildNestedSelect currently calls
buildConditionalSelect(obj, parentAliases) but buildConditionalSelect lowers
branches with buildNestedSelect(args[i * 2 + 1], parentAliases) which loses the
current select key/alias and prevents includes (toArray/IncludesSubquery) from
being produced; update the API so the destination/alias is threaded through:
add/pass a destination identifier (e.g. destKey or current select key) from
buildNestedSelect into buildConditionalSelect and ensure buildConditionalSelect
calls buildNestedSelect(branchValue, parentAliasesWithDest) (or
parentAliases.concat(dest)) when lowering each branch; adjust buildNestedSelect
signature and all call sites (including the initial call sites around
CaseWhenWrapper and the other occurrences noted) so branch values retain the
select alias and are treated as include nodes.
In `@packages/db/src/query/compiler/group-by.ts`:
- Around line 656-666: The conditionalSelect branch checker currently tests only
each branch's condition and value for aggregates but misses the optional
expr.defaultValue; update the logic in group-by.ts (inside the conditionalSelect
handling) to also call containsAggregate(expr.defaultValue) and include that
result in the overall some/any check so that aggregates present only in the
defaultValue are detected; reference the conditionalSelect `expr`, the helper
`containsAggregate`, and the branch structure when making this change.
---
Nitpick comments:
In `@packages/db/src/query/compiler/group-by.ts`:
- Around line 656-666: Replace the inline ad-hoc type assertion for conditional
selects with the actual ConditionalSelect IR type: import ConditionalSelect from
the IR module (or add it to the existing IR import), then narrow expr using that
type (e.g., treat expr as ConditionalSelect after the type check `expr.type ===
'conditionalSelect'`) and access expr.branches typed as
ConditionalSelect['branches'] instead of an anonymous cast; update the import
section at the top of group-by.ts accordingly and adjust the containsAggregate
checks to operate on the properly typed branch elements.
In `@packages/db/src/query/compiler/select.ts`:
- Around line 204-245: The truthiness logic in isCaseWhenConditionTrue is
duplicated; extract it into a single shared helper (e.g., export function
isCaseWhenConditionTrue from a new/shared utility module) and update
compileConditionalSelect to import and use that helper instead of its local
implementation, then update the other occurrence in compiler/evaluators.ts (and
any caseWhen implementations) to import the same helper so scalar and projection
caseWhen use identical logic; ensure the helper is exported with the same name
and update imports where isCaseWhenConditionTrue was previously defined or
referenced.
In `@packages/db/tests/query/case-when.test-d.ts`:
- Around line 31-49: The helper factory functions createUsers and createPosts
rely on inferred return types; add explicit return type annotations to both (for
example, the concrete collection type returned by createCollection for User and
Post or ReturnType<typeof createCollection> specialized for each) so the test
helpers have stable, precise return types; update the function signatures for
createUsers and createPosts to include those explicit return types.
In `@packages/db/tests/query/case-when.test.ts`:
- Around line 384-583: Add an assertion that explicitly covers the empty-include
corner case by ensuring a branch that should include child rows returns an empty
array when there are no matching child rows: modify one of the conditional
projection tests (e.g., the tests using
createUsersCollection/createPostsCollection and the LiveQuery named query) to
either include a user with no posts or adapt an existing user to have no posts,
call query.preload(), then assert that the included field (postTitles or
profile.posts) for that user is exactly [] using query.toArray (optionally
wrapped by stripVirtualPropsAndSymbols or childRows for Collection includes) so
the test verifies an empty-array include is handled correctly.
- Around line 59-77: Replace the unsafe any usages in the test helpers by typing
stripVirtualPropsAndSymbols(value: unknown): unknown (and arrays as unknown[])
and childRows(collection: { toArray: Iterable<unknown> }): unknown[]; keep the
existing runtime checks (Array.isArray, typeof value === 'object' && value !==
null) as the type guards, change the intermediate out to Record<string,
unknown>, and treat entries/returned values from stripVirtualProps as unknown
before recursing so you avoid any while preserving the current logic in
stripVirtualPropsAndSymbols, childRows, and the use of collection.toArray.
🪄 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: 069e5d1e-ffc5-4ecc-b6ca-f024c435c362
📒 Files selected for processing (14)
packages/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/select.tspackages/db/src/query/index.tspackages/db/src/query/ir.tspackages/db/src/query/live/collection-config-builder.tspackages/db/src/query/live/utils.tspackages/db/tests/query/case-when.test-d.tspackages/db/tests/query/case-when.test.ts
Thread conditional include destinations through caseWhen branches and tighten aggregate handling for grouped conditional projections. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/db/src/query/compiler/select.ts (1)
231-249: ⚡ Quick winExtract shared CASE truthiness evaluation.
isCaseWhenConditionTrue()is now duplicated here and inpackages/db/src/query/compiler/group-by.ts. Keeping both copies in sync is part ofcaseWhen()correctness now, so this should live in one shared helper.As per coding guidelines, Extract common logic into reusable utility functions when duplicated across multiple places.
🤖 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/select.ts` around lines 231 - 249, Duplicate CASE truthiness logic exists in isCaseWhenConditionTrue (in select.ts) and in group-by.ts; extract this logic into a single shared utility (e.g., create a new helper function name like evaluateCaseTruthiness or keep isCaseWhenConditionTrue) in a common utilities module and replace both local definitions with imports from that module, update references in caseWhen and any callers (select.ts's isCaseWhenConditionTrue and the duplicate in group-by.ts) to use the shared function, and remove the duplicated implementation so there is a single source of truth.
🤖 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/compiler/group-by.ts`:
- Around line 766-852: The branch conditions/values compiled in
compileGroupedConditionalSelect (and any grouped select/value compilation paths
used by compileGroupedSelectObject) still reference original namespaced refs
(e.g., users.status) but the grouped evaluator runs against a row containing
only group-backed namespaces like $selected/group-key, so refs must be lowered
before compiling; update compileGroupedConditionalSelect to transform
branch.condition and branch.value refs into group-scoped refs (or wrap them)
prior to calling compileExpression/compileGroupedSelectValue (i.e., add a helper
that rewrites BasicExpression PropRef nodes that point to grouped tables into
PropRefs under $selected or the group-key namespace and apply it to each
branch.condition and branch.value), and apply the same lowering when creating
entries in compileGroupedSelectObject for non-aggregate ref expressions so the
compiled functions will read from the reconstructed grouped row rather than
undefined original namespaces.
- Around line 746-759: The branch that handles isNestedSelectObject currently
blindly runs extractAndReplaceAggregates over every property, which converts
IncludesSubquery IR into a plain object and breaks compileGroupedSelectValue's
includesSubquery handling; fix this by detecting IncludesSubquery nodes while
iterating the object (check value.type === 'includesSubquery' or the project's
IncludesSubquery predicate) and preserve them as-is into transformed[key] (skip
calling extractAndReplaceAggregates), otherwise continue extracting aggregates
as before (keep using extractAndReplaceAggregates and merging into
allExtracted), so compileGroupedSelectValue can still match value.type ===
'includesSubquery'.
---
Nitpick comments:
In `@packages/db/src/query/compiler/select.ts`:
- Around line 231-249: Duplicate CASE truthiness logic exists in
isCaseWhenConditionTrue (in select.ts) and in group-by.ts; extract this logic
into a single shared utility (e.g., create a new helper function name like
evaluateCaseTruthiness or keep isCaseWhenConditionTrue) in a common utilities
module and replace both local definitions with imports from that module, update
references in caseWhen and any callers (select.ts's isCaseWhenConditionTrue and
the duplicate in group-by.ts) to use the shared function, and remove the
duplicated implementation so there is a single source of truth.
🪄 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: 40e73b2a-e80c-481d-930b-d0072bc10f93
📒 Files selected for processing (6)
.changeset/tender-mugs-hear.mdpackages/db/src/query/builder/functions.tspackages/db/src/query/builder/index.tspackages/db/src/query/compiler/group-by.tspackages/db/src/query/compiler/select.tspackages/db/tests/query/case-when.test.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/tender-mugs-hear.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/db/tests/query/case-when.test.ts
- packages/db/src/query/builder/functions.ts
Comment on lines
+746
to
+759
| if (isNestedSelectObject(expr)) { | ||
| const allExtracted: Record<string, Aggregate> = {} | ||
| const transformed: Select = {} | ||
|
|
||
| for (const [key, value] of Object.entries(expr)) { | ||
| const result = extractAndReplaceAggregates( | ||
| value as SelectValueExpression, | ||
| counter, | ||
| ) | ||
| Object.assign(allExtracted, result.extracted) | ||
| transformed[key] = result.transformed | ||
| } | ||
|
|
||
| return { transformed, extracted: allExtracted } |
Contributor
There was a problem hiding this comment.
Preserve IncludesSubquery nodes during aggregate extraction.
isNestedSelectObject() also matches IncludesSubquery, so this branch rewrites include IR into a plain object whenever the surrounding projection contains an aggregate. That prevents compileGroupedSelectValue() from hitting its value.type === 'includesSubquery' placeholder path and breaks grouped caseWhen() branches that mix aggregates with toArray(...) or collection includes.
Possible minimal fix
function extractAndReplaceAggregates(
expr: SelectValueExpression,
counter: { value: number },
): {
transformed: SelectValueExpression
extracted: Record<string, Aggregate>
} {
if (expr.type === `agg`) {
const alias = `__agg_${counter.value++}`
return {
transformed: new PropRef([`$selected`, alias]),
extracted: { [alias]: expr },
}
}
if (expr.type === `func`) {
...
}
+
+ if ((expr as { type?: string }).type === `includesSubquery`) {
+ return { transformed: expr, extracted: {} }
+ }
if (isConditionalSelect(expr)) {
...
}🤖 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/group-by.ts` around lines 746 - 759, The
branch that handles isNestedSelectObject currently blindly runs
extractAndReplaceAggregates over every property, which converts IncludesSubquery
IR into a plain object and breaks compileGroupedSelectValue's includesSubquery
handling; fix this by detecting IncludesSubquery nodes while iterating the
object (check value.type === 'includesSubquery' or the project's
IncludesSubquery predicate) and preserve them as-is into transformed[key] (skip
calling extractAndReplaceAggregates), otherwise continue extracting aggregates
as before (keep using extractAndReplaceAggregates and merging into
allExtracted), so compileGroupedSelectValue can still match value.type ===
'includesSubquery'.
Comment on lines
+766
to
+852
| function compileGroupedSelectValue( | ||
| value: SelectValueExpression, | ||
| ): (row: NamespacedRow) => any { | ||
| if (isConditionalSelect(value)) { | ||
| return compileGroupedConditionalSelect(value) | ||
| } | ||
|
|
||
| if (value.type === `includesSubquery`) { | ||
| return () => null | ||
| } | ||
|
|
||
| if (isNestedSelectObject(value)) { | ||
| return compileGroupedSelectObject(value) | ||
| } | ||
|
|
||
| if (!isExpressionLike(value)) { | ||
| return () => value | ||
| } | ||
|
|
||
| return compileExpression(value as BasicExpression) | ||
| } | ||
|
|
||
| function compileGroupedSelectObject( | ||
| obj: Select, | ||
| ): (row: NamespacedRow) => Record<string, any> { | ||
| const entries = Object.entries(obj).map(([key, value]) => { | ||
| if (key.startsWith(`__SPREAD_SENTINEL__`)) { | ||
| const rest = key.slice(`__SPREAD_SENTINEL__`.length) | ||
| const splitIndex = rest.lastIndexOf(`__`) | ||
| const pathStr = splitIndex >= 0 ? rest.slice(0, splitIndex) : rest | ||
| const isRefExpr = | ||
| typeof value === `object` && `type` in value && value.type === `ref` | ||
| const expression = isRefExpr | ||
| ? (value as BasicExpression) | ||
| : (new PropRef(pathStr.split(`.`)) as BasicExpression) | ||
|
|
||
| return { | ||
| key, | ||
| spread: true, | ||
| value: compileExpression(expression), | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| key, | ||
| spread: false, | ||
| value: compileGroupedSelectValue(value as SelectValueExpression), | ||
| } | ||
| }) | ||
|
|
||
| return (row) => { | ||
| const result: Record<string, any> = {} | ||
| for (const entry of entries) { | ||
| const value = entry.value(row) | ||
| if (entry.spread) { | ||
| if (value && typeof value === `object`) { | ||
| Object.assign(result, value) | ||
| } | ||
| } else { | ||
| result[entry.key] = value | ||
| } | ||
| } | ||
| return result | ||
| } | ||
| } | ||
|
|
||
| function compileGroupedConditionalSelect( | ||
| conditional: ConditionalSelect, | ||
| ): (row: NamespacedRow) => any { | ||
| const branches = conditional.branches.map((branch) => ({ | ||
| condition: compileExpression(branch.condition), | ||
| value: compileGroupedSelectValue(branch.value), | ||
| })) | ||
| const defaultValue = | ||
| conditional.defaultValue === undefined | ||
| ? undefined | ||
| : compileGroupedSelectValue(conditional.defaultValue) | ||
|
|
||
| return (row) => { | ||
| for (const branch of branches) { | ||
| if (isCaseWhenConditionTrue(branch.condition(row))) { | ||
| return branch.value(row) | ||
| } | ||
| } | ||
|
|
||
| return defaultValue ? defaultValue(row) : undefined | ||
| } |
Contributor
There was a problem hiding this comment.
Grouped conditional wrappers lose access to grouped refs.
These evaluators compile branch conditions/values with refs like users.status intact, but wrapped aggregate evaluation still runs them against { $selected } only. A grouped projection such as caseWhen(eq(users.status, 'active'), count(users.id), 0) will therefore see users.status as undefined and take the wrong branch/value. Lower the non-aggregate group-backed refs to $selected/group-key refs before compiling, or evaluate the wrapper against a row that reconstructs the grouped namespaces.
🤖 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/group-by.ts` around lines 766 - 852, The
branch conditions/values compiled in compileGroupedConditionalSelect (and any
grouped select/value compilation paths used by compileGroupedSelectObject) still
reference original namespaced refs (e.g., users.status) but the grouped
evaluator runs against a row containing only group-backed namespaces like
$selected/group-key, so refs must be lowered before compiling; update
compileGroupedConditionalSelect to transform branch.condition and branch.value
refs into group-scoped refs (or wrap them) prior to calling
compileExpression/compileGroupedSelectValue (i.e., add a helper that rewrites
BasicExpression PropRef nodes that point to grouped tables into PropRefs under
$selected or the group-key namespace and apply it to each branch.condition and
branch.value), and apply the same lowering when creating entries in
compileGroupedSelectObject for non-aggregate ref expressions so the compiled
functions will read from the reconstructed grouped row rather than undefined
original namespaces.
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
1 participant
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.
Summary
caseWhen(...)as a unified conditional query helper inspired by SQLCASE WHEN, with scalar expression support inselect,where,orderBy,groupBy,having, and equality join operands.caseWhensupport forselect()branch values, including nested projection objects, ref spreads,toArray(...), and Collection includes.caseWhenbranches are not materialized for that parent row.caseWhenin the live query guide and adds a changeset for the new operator.Why This Is Useful
caseWhenlets queries express conditional logic without dropping down to functional callbacks, so the query compiler can keep optimizing and incrementally maintaining the result.A common scalar use case is categorizing rows while preserving literal result types:
Because scalar
caseWhenis a normal query expression, it can also drive filtering, grouping, sorting, and join operands:Projection-valued branches make optional result shapes easier to model. For example, a joined row can project an optional nested object only when the joined source exists:
It also enables guarded includes, where child collections are materialized only for rows whose branch is active:
Design
caseWhenis one public API with two internal paths selected by branch value shape, not call location:Func('caseWhen', ...). The evaluator walks condition/value pairs left to right, returns the first matching branch, evaluates only the active value branch, and returnsnullwhen there is no default.CaseWhenWrapperthat is lowered byselect()intoConditionalSelectIR. The select compiler evaluates branch conditions lazily and returnsundefinedwhen there is no matching projection branch and no default.ConditionalSelectbranches carry guard metadata through include extraction/routing. Parent keys are emitted only when the guard chain matches, which keeps inactive branch includes from loading or attaching.Full design notes: https://gist.github.com/samwillis/60d8d0d937cf23267d0f8383bea3af92
Test plan
pnpm exec vitest --run tests/query/case-when.test.ts tests/query/case-when.test-d.tsfrompackages/dbpnpm testfrompackages/dbpnpm testfrom the repo root was also run;packages/dbpassed, but the workspace run failed in unrelated@tanstack/expo-db-sqlite-persistencetype-checking for untrackedtests/helpers/expo-runtime-node-preload.cjs.Made with Cursor
Summary by CodeRabbit
New Features
Includes / Materialization
Tests
Made with Cursor