Fix serializing null/undefined when generating subset queries #951
+466
−0
Conversation
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
When comparison operators (eq, gt, lt, etc.) were used with null/undefined
values, the SQL compiler would generate placeholders ($1, $2) in the WHERE
clause but skip adding the params to the dictionary because serialize()
returns empty string for null/undefined.
This resulted in invalid queries being sent to Electric like:
subset__where="name" = $1
subset__params={}
The fix:
- For eq(col, null): Transform to "col IS NULL" syntax
- For other comparisons (gt, lt, gte, lte, like, ilike): Throw a clear
error since null comparisons don't make semantic sense in SQL
Added comprehensive tests for the sql-compiler including null handling.
🦋 Changeset detectedLatest commit: 9401b42 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
More templates
@tanstack/angular-db
@tanstack/db
@tanstack/db-ivm
@tanstack/electric-db-collection
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
Contributor
|
Size Change: 0 B Total Size: 87.1 kB ℹ️ View Unchanged
|
Contributor
|
Size Change: 0 B Total Size: 3.35 kB ℹ️ View Unchanged
|
Address reviewer feedback: - eq(null, null) now throws (both args null would cause missing params) - eq(null, literal) now throws (comparing literal to null is nonsensical) - Only allow ref and func types as non-null arg in eq(..., null) - Update changeset to explicitly mention undefined behavior - Add tests for edge cases and OR + null equality
…antics When eq() is called with a literal null/undefined value, the local JavaScript evaluator now treats it as an IS NULL check instead of using 3-valued logic (which would always return UNKNOWN). This matches the SQL compiler's transformation of eq(col, null) to IS NULL, ensuring consistent behavior between local query evaluation and remote SQL queries. - eq(col, null) now returns true if col is null/undefined, false otherwise - eq(null, col) is also handled symmetrically - eq(null, null) returns true (both are null) - 3-valued logic still applies for column-to-column comparisons This fixes e2e test failures where eq(col, null) queries returned 0 results because all rows were being excluded by the UNKNOWN result.
Add design document explaining the middle-ground approach for handling eq(col, null) in the context of PR #765's 3-valued logic implementation. Key points: - Literal null values in eq() are transformed to isNull semantics - 3-valued logic still applies for column-to-column comparisons - This maintains SQL/JS consistency and handles dynamic values gracefully
Per Kevin's feedback on the 3-valued logic design (PR #765), eq(col, null) should throw an error rather than being transformed to IS NULL. This is consistent with how other comparison operators (gt, lt, etc.) handle null. Changes: - Revert JS evaluator change that transformed eq(col, null) to isNull semantics - Update SQL compiler to throw error for eq(col, null) instead of IS NULL - Update all related unit tests to expect errors - Remove e2e tests for eq(col, null) (now throws error) - Update documentation to explain the correct approach Users should: - Use isNull(col) or isUndefined(col) to check for null values - Handle dynamic null values explicitly in their code - Use non-nullable columns for cursor-based pagination The original bug (invalid SQL with missing params) is fixed by throwing a clear error that guides users to the correct approach.
kevin-dp
approved these changes
Dec 4, 2025
| @@ -0,0 +1,105 @@ | |||
| # Handling Null Values in Comparison Operators | |||
Contributor
There was a problem hiding this comment.
Is this file intended to be merged into the codebase or is this cursor/claude writing an md file to tell us what it did but not actually intended to be merged?
Collaborator
Author
There was a problem hiding this comment.
yeah telling us what it did — I wanted a more detailed doc for you to look at
|
|
||
| Now: | ||
|
|
||
| - `eq(col, null)` and `eq(col, undefined)` transform to `"col" IS NULL` syntax |
Contributor
There was a problem hiding this comment.
Where is this transformation happening? I don't see it in the changes?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When comparison operators (eq, gt, lt, etc.) were used with null/undefined values, the SQL compiler would generate placeholders ($1, $2) in the WHERE clause but skip adding the params to the dictionary because serialize() returns empty string for null/undefined.
This resulted in invalid queries being sent to Electric like:
subset__where="name" = $1
subset__params={}
The fix:
Added comprehensive tests for the sql-compiler including null handling.
🎯 Changes
✅ Checklist
pnpm test:pr.🚀 Release Impact