From 90889ae3c21a350ed66c28ff8c9ec8e8c56bbae3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Dec 2025 21:01:47 +0000 Subject: [PATCH 1/9] Fix invalid Electric proxy queries with missing params for null values 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. --- .../src/sql-compiler.ts | 47 ++++ .../tests/sql-compiler.test.ts | 265 ++++++++++++++++++ 2 files changed, 312 insertions(+) create mode 100644 packages/electric-db-collection/tests/sql-compiler.test.ts diff --git a/packages/electric-db-collection/src/sql-compiler.ts b/packages/electric-db-collection/src/sql-compiler.ts index c941c7fef..bd36e7cee 100644 --- a/packages/electric-db-collection/src/sql-compiler.ts +++ b/packages/electric-db-collection/src/sql-compiler.ts @@ -124,6 +124,13 @@ function compileOrderByClause( return sql } +/** + * Check if a BasicExpression represents a null/undefined value + */ +function isNullValue(exp: IR.BasicExpression): boolean { + return exp.type === `val` && (exp.value === null || exp.value === undefined) +} + function compileFunction( exp: IR.Func, params: Array = [] @@ -132,6 +139,36 @@ function compileFunction( const opName = getOpName(name) + // Handle comparison operators with null/undefined values + // These would create invalid queries with missing params (e.g., "col = $1" with empty params) + if (isComparisonOp(name)) { + const nullArgIndex = args.findIndex((arg: IR.BasicExpression) => + isNullValue(arg) + ) + + if (nullArgIndex !== -1) { + const otherArgIndex = nullArgIndex === 0 ? 1 : 0 + const otherArg = args[otherArgIndex] + + if (name === `eq`) { + // Transform eq(col, null) to "col IS NULL" + // This is the semantically correct behavior in SQL + if (otherArg) { + return `${compileBasicExpression(otherArg, params)} IS NULL` + } + } + + // For other comparison operators (gt, lt, gte, lte, like, ilike), + // null comparisons don't make semantic sense in SQL (they always evaluate to UNKNOWN) + throw new Error( + `Cannot use null/undefined value with '${name}' operator. ` + + `Comparisons with null always evaluate to UNKNOWN in SQL. ` + + `Use isNull() or isUndefined() to check for null values, ` + + `or filter out null values before building the query.` + ) + } + } + const compiledArgs = args.map((arg: IR.BasicExpression) => compileBasicExpression(arg, params) ) @@ -198,6 +235,16 @@ function isBinaryOp(name: string): boolean { return binaryOps.includes(name) } +/** + * Check if operator is a comparison operator that takes two values + * These operators cannot accept null/undefined as values + * (null comparisons in SQL always evaluate to UNKNOWN) + */ +function isComparisonOp(name: string): boolean { + const comparisonOps = [`eq`, `gt`, `gte`, `lt`, `lte`, `like`, `ilike`] + return comparisonOps.includes(name) +} + function getOpName(name: string): string { const opNames = { eq: `=`, diff --git a/packages/electric-db-collection/tests/sql-compiler.test.ts b/packages/electric-db-collection/tests/sql-compiler.test.ts new file mode 100644 index 000000000..c7add31e3 --- /dev/null +++ b/packages/electric-db-collection/tests/sql-compiler.test.ts @@ -0,0 +1,265 @@ +import { describe, expect, it } from "vitest" +import { compileSQL } from "../src/sql-compiler" + +// Helper to create a value expression +function val(value: T) { + return { type: `val` as const, value } +} + +// Helper to create a reference expression +function ref(...path: Array) { + return { type: `ref` as const, path } +} + +// Helper to create a function expression +function func(name: string, args: Array) { + return { type: `func` as const, name, args } +} + +describe(`sql-compiler`, () => { + describe(`compileSQL`, () => { + describe(`basic where clauses`, () => { + it(`should compile eq with string value`, () => { + const result = compileSQL({ + where: func(`eq`, [ref(`name`), val(`John`)]), + }) + expect(result.where).toBe(`"name" = $1`) + expect(result.params).toEqual({ "1": `John` }) + }) + + it(`should compile eq with number value`, () => { + const result = compileSQL({ + where: func(`eq`, [ref(`age`), val(25)]), + }) + expect(result.where).toBe(`"age" = $1`) + expect(result.params).toEqual({ "1": `25` }) + }) + + it(`should compile gt operator`, () => { + const result = compileSQL({ + where: func(`gt`, [ref(`age`), val(18)]), + }) + expect(result.where).toBe(`"age" > $1`) + expect(result.params).toEqual({ "1": `18` }) + }) + + it(`should compile lt operator`, () => { + const result = compileSQL({ + where: func(`lt`, [ref(`price`), val(100)]), + }) + expect(result.where).toBe(`"price" < $1`) + expect(result.params).toEqual({ "1": `100` }) + }) + + it(`should compile gte operator`, () => { + const result = compileSQL({ + where: func(`gte`, [ref(`quantity`), val(10)]), + }) + expect(result.where).toBe(`"quantity" >= $1`) + expect(result.params).toEqual({ "1": `10` }) + }) + + it(`should compile lte operator`, () => { + const result = compileSQL({ + where: func(`lte`, [ref(`rating`), val(5)]), + }) + expect(result.where).toBe(`"rating" <= $1`) + expect(result.params).toEqual({ "1": `5` }) + }) + }) + + describe(`compound where clauses`, () => { + it(`should compile AND with two conditions`, () => { + const result = compileSQL({ + where: func(`and`, [ + func(`eq`, [ref(`projectId`), val(`uuid-123`)]), + func(`gt`, [ref(`name`), val(`cursor-value`)]), + ]), + }) + // Note: 2-arg AND doesn't add parentheses around the operands + expect(result.where).toBe(`"projectId" = $1 AND "name" > $2`) + expect(result.params).toEqual({ "1": `uuid-123`, "2": `cursor-value` }) + }) + + it(`should compile AND with more than two conditions`, () => { + const result = compileSQL({ + where: func(`and`, [ + func(`eq`, [ref(`a`), val(`1`)]), + func(`eq`, [ref(`b`), val(`2`)]), + func(`eq`, [ref(`c`), val(`3`)]), + ]), + }) + // >2 args adds parentheses + expect(result.where).toBe(`("a" = $1) AND ("b" = $2) AND ("c" = $3)`) + expect(result.params).toEqual({ "1": `1`, "2": `2`, "3": `3` }) + }) + + it(`should compile OR with two conditions`, () => { + const result = compileSQL({ + where: func(`or`, [ + func(`eq`, [ref(`status`), val(`active`)]), + func(`eq`, [ref(`status`), val(`pending`)]), + ]), + }) + expect(result.where).toBe(`"status" = $1 OR "status" = $2`) + expect(result.params).toEqual({ "1": `active`, "2": `pending` }) + }) + }) + + describe(`null/undefined value handling`, () => { + it(`should transform eq(col, null) to IS NULL`, () => { + const result = compileSQL({ + where: func(`eq`, [ref(`deletedAt`), val(null)]), + }) + expect(result.where).toBe(`"deletedAt" IS NULL`) + expect(result.params).toEqual({}) + }) + + it(`should transform eq(col, undefined) to IS NULL`, () => { + const result = compileSQL({ + where: func(`eq`, [ref(`deletedAt`), val(undefined)]), + }) + expect(result.where).toBe(`"deletedAt" IS NULL`) + expect(result.params).toEqual({}) + }) + + it(`should transform eq(null, col) to IS NULL (reversed order)`, () => { + const result = compileSQL({ + where: func(`eq`, [val(null), ref(`name`)]), + }) + expect(result.where).toBe(`"name" IS NULL`) + expect(result.params).toEqual({}) + }) + + it(`should throw error for gt with null value`, () => { + expect(() => + compileSQL({ + where: func(`gt`, [ref(`age`), val(null)]), + }) + ).toThrow(`Cannot use null/undefined value with 'gt' operator`) + }) + + it(`should throw error for lt with undefined value`, () => { + expect(() => + compileSQL({ + where: func(`lt`, [ref(`age`), val(undefined)]), + }) + ).toThrow(`Cannot use null/undefined value with 'lt' operator`) + }) + + it(`should throw error for gte with null value`, () => { + expect(() => + compileSQL({ + where: func(`gte`, [ref(`price`), val(null)]), + }) + ).toThrow(`Cannot use null/undefined value with 'gte' operator`) + }) + + it(`should throw error for lte with null value`, () => { + expect(() => + compileSQL({ + where: func(`lte`, [ref(`rating`), val(null)]), + }) + ).toThrow(`Cannot use null/undefined value with 'lte' operator`) + }) + + it(`should throw error for like with null value`, () => { + expect(() => + compileSQL({ + where: func(`like`, [ref(`name`), val(null)]), + }) + ).toThrow(`Cannot use null/undefined value with 'like' operator`) + }) + + it(`should throw error for ilike with null value`, () => { + expect(() => + compileSQL({ + where: func(`ilike`, [ref(`name`), val(null)]), + }) + ).toThrow(`Cannot use null/undefined value with 'ilike' operator`) + }) + + it(`should handle eq(col, null) in AND clause with two conditions`, () => { + const result = compileSQL({ + where: func(`and`, [ + func(`eq`, [ref(`projectId`), val(`uuid-123`)]), + func(`eq`, [ref(`deletedAt`), val(null)]), + ]), + }) + expect(result.where).toBe(`"projectId" = $1 AND "deletedAt" IS NULL`) + expect(result.params).toEqual({ "1": `uuid-123` }) + }) + + it(`should handle mixed null and value conditions in AND with >2 conditions`, () => { + const result = compileSQL({ + where: func(`and`, [ + func(`eq`, [ref(`status`), val(`active`)]), + func(`eq`, [ref(`archivedAt`), val(null)]), + func(`gt`, [ref(`createdAt`), val(`2024-01-01`)]), + ]), + }) + expect(result.where).toBe( + `("status" = $1) AND ("archivedAt" IS NULL) AND ("createdAt" > $2)` + ) + expect(result.params).toEqual({ "1": `active`, "2": `2024-01-01` }) + }) + + it(`should not include params for null values in complex queries`, () => { + // This test simulates the bug scenario: a query with both valid params and null + // Before the fix, this would generate: + // where: "projectId" = $1 AND "name" > $2 + // params: { "1": "uuid" } // missing $2! + // After the fix, gt(name, null) throws an error + expect(() => + compileSQL({ + where: func(`and`, [ + func(`eq`, [ref(`projectId`), val(`uuid`)]), + func(`gt`, [ref(`name`), val(null)]), + ]), + }) + ).toThrow(`Cannot use null/undefined value with 'gt' operator`) + }) + }) + + describe(`isNull/isUndefined operators`, () => { + it(`should compile isNull correctly`, () => { + const result = compileSQL({ + where: func(`isNull`, [ref(`deletedAt`)]), + }) + expect(result.where).toBe(`"deletedAt" IS NULL`) + expect(result.params).toEqual({}) + }) + + it(`should compile isUndefined correctly`, () => { + const result = compileSQL({ + where: func(`isUndefined`, [ref(`field`)]), + }) + expect(result.where).toBe(`"field" IS NULL`) + expect(result.params).toEqual({}) + }) + + it(`should compile NOT isNull correctly`, () => { + const result = compileSQL({ + where: func(`not`, [func(`isNull`, [ref(`name`)])]), + }) + expect(result.where).toBe(`"name" IS NOT NULL`) + expect(result.params).toEqual({}) + }) + }) + + describe(`empty where clause`, () => { + it(`should add true = true when no where clause`, () => { + const result = compileSQL({}) + expect(result.where).toBe(`true = true`) + expect(result.params).toEqual({}) + }) + }) + + describe(`limit`, () => { + it(`should include limit in result`, () => { + const result = compileSQL({ limit: 10 }) + expect(result.limit).toBe(10) + }) + }) + }) +}) From 0f4c7bcf8bf177571dd9b033399f034dfeda7721 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Dec 2025 21:05:35 +0000 Subject: [PATCH 2/9] chore: add changeset for Electric null params fix --- .changeset/fix-electric-null-params.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .changeset/fix-electric-null-params.md diff --git a/.changeset/fix-electric-null-params.md b/.changeset/fix-electric-null-params.md new file mode 100644 index 000000000..01a2eb5b1 --- /dev/null +++ b/.changeset/fix-electric-null-params.md @@ -0,0 +1,11 @@ +--- +"@tanstack/electric-db-collection": patch +--- + +Fix invalid Electric proxy queries with missing params for null/undefined values + +When comparison operators 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. This resulted in invalid queries being sent to Electric. + +Now: +- `eq(col, null)` transforms to `"col" IS NULL` syntax +- Other comparisons (gt, lt, gte, lte, like, ilike) with null throw a clear error since null comparisons don't make semantic sense in SQL From 3ba00ec8abe827f4bdb30f097f0d9f7dd36df84f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Dec 2025 21:24:40 +0000 Subject: [PATCH 3/9] fix: use type assertions in sql-compiler tests for phantom type compatibility --- .changeset/fix-electric-null-params.md | 1 + .../tests/sql-compiler.test.ts | 14 ++++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.changeset/fix-electric-null-params.md b/.changeset/fix-electric-null-params.md index 01a2eb5b1..258acb845 100644 --- a/.changeset/fix-electric-null-params.md +++ b/.changeset/fix-electric-null-params.md @@ -7,5 +7,6 @@ Fix invalid Electric proxy queries with missing params for null/undefined values When comparison operators 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. This resulted in invalid queries being sent to Electric. Now: + - `eq(col, null)` transforms to `"col" IS NULL` syntax - Other comparisons (gt, lt, gte, lte, like, ilike) with null throw a clear error since null comparisons don't make semantic sense in SQL diff --git a/packages/electric-db-collection/tests/sql-compiler.test.ts b/packages/electric-db-collection/tests/sql-compiler.test.ts index c7add31e3..111bb7761 100644 --- a/packages/electric-db-collection/tests/sql-compiler.test.ts +++ b/packages/electric-db-collection/tests/sql-compiler.test.ts @@ -1,19 +1,21 @@ import { describe, expect, it } from "vitest" import { compileSQL } from "../src/sql-compiler" +import type { IR } from "@tanstack/db" // Helper to create a value expression -function val(value: T) { - return { type: `val` as const, value } +function val(value: T): IR.BasicExpression { + return { type: `val`, value } as IR.BasicExpression } // Helper to create a reference expression -function ref(...path: Array) { - return { type: `ref` as const, path } +function ref(...path: Array): IR.BasicExpression { + return { type: `ref`, path } as IR.BasicExpression } // Helper to create a function expression -function func(name: string, args: Array) { - return { type: `func` as const, name, args } + +function func(name: string, args: Array): IR.BasicExpression { + return { type: `func`, name, args } as IR.BasicExpression } describe(`sql-compiler`, () => { From d3448384d93f2b03c85f636c1f7061093582b5f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 3 Dec 2025 00:04:47 +0000 Subject: [PATCH 4/9] fix: handle edge cases for null comparisons in sql-compiler 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 --- .changeset/fix-electric-null-params.md | 4 +-- .../src/sql-compiler.ts | 16 ++++++---- .../tests/sql-compiler.test.ts | 29 +++++++++++++++++++ 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/.changeset/fix-electric-null-params.md b/.changeset/fix-electric-null-params.md index 258acb845..b97c395e3 100644 --- a/.changeset/fix-electric-null-params.md +++ b/.changeset/fix-electric-null-params.md @@ -8,5 +8,5 @@ When comparison operators were used with null/undefined values, the SQL compiler Now: -- `eq(col, null)` transforms to `"col" IS NULL` syntax -- Other comparisons (gt, lt, gte, lte, like, ilike) with null throw a clear error since null comparisons don't make semantic sense in SQL +- `eq(col, null)` and `eq(col, undefined)` transform to `"col" IS NULL` syntax +- Other comparisons (gt, lt, gte, lte, like, ilike) with null/undefined throw a clear error since null comparisons don't make semantic sense in SQL diff --git a/packages/electric-db-collection/src/sql-compiler.ts b/packages/electric-db-collection/src/sql-compiler.ts index bd36e7cee..8ad9c4700 100644 --- a/packages/electric-db-collection/src/sql-compiler.ts +++ b/packages/electric-db-collection/src/sql-compiler.ts @@ -150,15 +150,21 @@ function compileFunction( const otherArgIndex = nullArgIndex === 0 ? 1 : 0 const otherArg = args[otherArgIndex] - if (name === `eq`) { - // Transform eq(col, null) to "col IS NULL" + if ( + name === `eq` && + otherArg && + !isNullValue(otherArg) && + (otherArg.type === `ref` || otherArg.type === `func`) + ) { + // Transform eq(col, null/undefined) or eq(func(...), null/undefined) to "expr IS NULL" // This is the semantically correct behavior in SQL - if (otherArg) { - return `${compileBasicExpression(otherArg, params)} IS NULL` - } + // We only allow ref and func types - comparing a literal to null (e.g., eq(42, null)) + // is almost certainly a mistake and should throw + return `${compileBasicExpression(otherArg, params)} IS NULL` } // For other comparison operators (gt, lt, gte, lte, like, ilike), + // or eq where both args are null/undefined, // null comparisons don't make semantic sense in SQL (they always evaluate to UNKNOWN) throw new Error( `Cannot use null/undefined value with '${name}' operator. ` + diff --git a/packages/electric-db-collection/tests/sql-compiler.test.ts b/packages/electric-db-collection/tests/sql-compiler.test.ts index 111bb7761..5b6503789 100644 --- a/packages/electric-db-collection/tests/sql-compiler.test.ts +++ b/packages/electric-db-collection/tests/sql-compiler.test.ts @@ -221,6 +221,35 @@ describe(`sql-compiler`, () => { }) ).toThrow(`Cannot use null/undefined value with 'gt' operator`) }) + + it(`should throw error for eq(null, null)`, () => { + // Both args are null - this is nonsensical and would cause missing params + expect(() => + compileSQL({ + where: func(`eq`, [val(null), val(null)]), + }) + ).toThrow(`Cannot use null/undefined value with 'eq' operator`) + }) + + it(`should throw error for eq(null, literal)`, () => { + // Comparing null to a literal is nonsensical (always evaluates to UNKNOWN) + expect(() => + compileSQL({ + where: func(`eq`, [val(null), val(42)]), + }) + ).toThrow(`Cannot use null/undefined value with 'eq' operator`) + }) + + it(`should handle eq(col, null) in OR clause`, () => { + const result = compileSQL({ + where: func(`or`, [ + func(`eq`, [ref(`deletedAt`), val(null)]), + func(`eq`, [ref(`status`), val(`active`)]), + ]), + }) + expect(result.where).toBe(`"deletedAt" IS NULL OR "status" = $1`) + expect(result.params).toEqual({ "1": `active` }) + }) }) describe(`isNull/isUndefined operators`, () => { From 222d529bc991b72ab782f2e792a5c3e1afb8ea83 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 3 Dec 2025 00:23:46 +0000 Subject: [PATCH 5/9] test: add e2e tests for eq(col, null) transformation to IS NULL --- .../src/suites/predicates.suite.ts | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/packages/db-collection-e2e/src/suites/predicates.suite.ts b/packages/db-collection-e2e/src/suites/predicates.suite.ts index a4f446856..d687eea9d 100644 --- a/packages/db-collection-e2e/src/suites/predicates.suite.ts +++ b/packages/db-collection-e2e/src/suites/predicates.suite.ts @@ -702,6 +702,79 @@ export function createPredicatesTestSuite( await query.cleanup() }) + it(`should filter with eq(col, null) same as isNull()`, async () => { + // eq(col, null) should be transformed to IS NULL and work correctly + const config = await getConfig() + const usersCollection = config.collections.onDemand.users + + const query = createLiveQueryCollection((q) => + q + .from({ user: usersCollection }) + .where(({ user }) => eq(user.email, null)) + ) + + await query.preload() + await waitForQueryData(query, { minSize: 1 }) + + const results = Array.from(query.state.values()) + expect(results.length).toBeGreaterThan(0) + assertAllItemsMatch(query, (u) => u.email === null) + + await query.cleanup() + }) + + it(`should filter with eq(col, null) in AND clause`, async () => { + // Tests that eq(col, null) works correctly when combined with other predicates + const config = await getConfig() + const usersCollection = config.collections.onDemand.users + + const query = createLiveQueryCollection((q) => + q + .from({ user: usersCollection }) + .where(({ user }) => + and(eq(user.email, null), eq(user.isActive, true)) + ) + ) + + await query.preload() + await waitForQueryData(query, { minSize: 1 }) + + const results = Array.from(query.state.values()) + expect(results.length).toBeGreaterThan(0) + assertAllItemsMatch( + query, + (u) => u.email === null && u.isActive === true + ) + + await query.cleanup() + }) + + it(`should filter with eq(col, null) in OR clause`, async () => { + // Tests that eq(col, null) works correctly in OR with other predicates + const config = await getConfig() + const usersCollection = config.collections.onDemand.users + + const query = createLiveQueryCollection((q) => + q + .from({ user: usersCollection }) + .where(({ user }) => + or(eq(user.email, null), eq(user.name, `Alice 0`)) + ) + ) + + await query.preload() + await waitForQueryData(query, { minSize: 1 }) + + const results = Array.from(query.state.values()) + expect(results.length).toBeGreaterThan(0) + assertAllItemsMatch( + query, + (u) => u.email === null || u.name === `Alice 0` + ) + + await query.cleanup() + }) + it(`should filter with not(isNull()) on nullable field`, async () => { const config = await getConfig() const usersCollection = config.collections.onDemand.users From 3c839d65172e5036f6c9f9e778ff5931f4d2b4e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 3 Dec 2025 00:57:12 +0000 Subject: [PATCH 6/9] test: improve e2e tests for eq(col, null) with longer timeout and baseline comparison --- .../src/suites/predicates.suite.ts | 18 ++++++++++++++++-- .../tests/sql-compiler.test.ts | 14 ++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/packages/db-collection-e2e/src/suites/predicates.suite.ts b/packages/db-collection-e2e/src/suites/predicates.suite.ts index d687eea9d..ac572114c 100644 --- a/packages/db-collection-e2e/src/suites/predicates.suite.ts +++ b/packages/db-collection-e2e/src/suites/predicates.suite.ts @@ -707,6 +707,18 @@ export function createPredicatesTestSuite( const config = await getConfig() const usersCollection = config.collections.onDemand.users + // First verify the existing isNull test pattern works + const isNullQuery = createLiveQueryCollection((q) => + q + .from({ user: usersCollection }) + .where(({ user }) => isNull(user.email)) + ) + await isNullQuery.preload() + await waitForQueryData(isNullQuery, { minSize: 1, timeout: 5000 }) + const isNullResults = Array.from(isNullQuery.state.values()) + await isNullQuery.cleanup() + + // Now test eq(col, null) produces same results const query = createLiveQueryCollection((q) => q .from({ user: usersCollection }) @@ -714,10 +726,12 @@ export function createPredicatesTestSuite( ) await query.preload() - await waitForQueryData(query, { minSize: 1 }) + await waitForQueryData(query, { minSize: 1, timeout: 5000 }) const results = Array.from(query.state.values()) expect(results.length).toBeGreaterThan(0) + // Should return same count as isNull query + expect(results.length).toBe(isNullResults.length) assertAllItemsMatch(query, (u) => u.email === null) await query.cleanup() @@ -737,7 +751,7 @@ export function createPredicatesTestSuite( ) await query.preload() - await waitForQueryData(query, { minSize: 1 }) + await waitForQueryData(query, { minSize: 1, timeout: 5000 }) const results = Array.from(query.state.values()) expect(results.length).toBeGreaterThan(0) diff --git a/packages/electric-db-collection/tests/sql-compiler.test.ts b/packages/electric-db-collection/tests/sql-compiler.test.ts index 5b6503789..8b956f8fd 100644 --- a/packages/electric-db-collection/tests/sql-compiler.test.ts +++ b/packages/electric-db-collection/tests/sql-compiler.test.ts @@ -240,6 +240,20 @@ describe(`sql-compiler`, () => { ).toThrow(`Cannot use null/undefined value with 'eq' operator`) }) + it(`should produce identical output for eq(col, null) and isNull(col)`, () => { + // Ensure eq(col, null) produces exactly the same SQL as isNull(col) + const eqResult = compileSQL({ + where: func(`eq`, [ref(`email`), val(null)]), + }) + const isNullResult = compileSQL({ + where: func(`isNull`, [ref(`email`)]), + }) + expect(eqResult.where).toBe(isNullResult.where) + expect(eqResult.params).toEqual(isNullResult.params) + expect(eqResult.where).toBe(`"email" IS NULL`) + expect(eqResult.params).toEqual({}) + }) + it(`should handle eq(col, null) in OR clause`, () => { const result = compileSQL({ where: func(`or`, [ From 30063585bc165c7191e20d3394fd0f88d53559bc Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 3 Dec 2025 01:26:41 +0000 Subject: [PATCH 7/9] fix: handle eq(col, null) in local evaluator to match SQL IS NULL semantics 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. --- packages/db/src/query/compiler/evaluators.ts | 32 ++++++++++ .../tests/query/compiler/evaluators.test.ts | 60 +++++++++++++++---- 2 files changed, 80 insertions(+), 12 deletions(-) diff --git a/packages/db/src/query/compiler/evaluators.ts b/packages/db/src/query/compiler/evaluators.ts index 19c69663d..1b1476a5c 100644 --- a/packages/db/src/query/compiler/evaluators.ts +++ b/packages/db/src/query/compiler/evaluators.ts @@ -163,6 +163,38 @@ function compileFunction(func: Func, isSingleRow: boolean): (data: any) => any { switch (func.name) { // Comparison operators case `eq`: { + // Check at compile time if one argument is a literal null/undefined value + // If so, transform to isNull semantics (return true/false, not UNKNOWN) + // This matches SQL behavior where eq(col, null) is transformed to IS NULL + const argExprA = func.args[0] + const argExprB = func.args[1] + const aIsLiteralNull = + argExprA?.type === `val` && + (argExprA.value === null || argExprA.value === undefined) + const bIsLiteralNull = + argExprB?.type === `val` && + (argExprB.value === null || argExprB.value === undefined) + + if (aIsLiteralNull && bIsLiteralNull) { + // eq(null, null) or eq(undefined, undefined) - always true + return () => true + } else if (aIsLiteralNull) { + // eq(null, col) - check if col value is null/undefined + const argB = compiledArgs[1]! + return (data) => { + const b = argB(data) + return b === null || b === undefined + } + } else if (bIsLiteralNull) { + // eq(col, null) - check if col value is null/undefined + const argA = compiledArgs[0]! + return (data) => { + const a = argA(data) + return a === null || a === undefined + } + } + + // Standard equality comparison (no literal nulls) const argA = compiledArgs[0]! const argB = compiledArgs[1]! return (data) => { diff --git a/packages/db/tests/query/compiler/evaluators.test.ts b/packages/db/tests/query/compiler/evaluators.test.ts index 64a867196..a9412c3f7 100644 --- a/packages/db/tests/query/compiler/evaluators.test.ts +++ b/packages/db/tests/query/compiler/evaluators.test.ts @@ -378,36 +378,72 @@ describe(`evaluators`, () => { describe(`comparison operators`, () => { describe(`eq (equality)`, () => { - it(`handles eq with null and null (3-valued logic)`, () => { + it(`handles eq with literal null and literal null (IS NULL semantics)`, () => { const func = new Func(`eq`, [new Value(null), new Value(null)]) const compiled = compileExpression(func) - // In 3-valued logic, null = null returns UNKNOWN (null) - expect(compiled({})).toBe(null) + // eq(null, null) returns true - both nulls are equal in IS NULL semantics + expect(compiled({})).toBe(true) }) - it(`handles eq with null and value (3-valued logic)`, () => { + it(`handles eq with literal null and literal value (IS NULL semantics)`, () => { const func = new Func(`eq`, [new Value(null), new Value(5)]) const compiled = compileExpression(func) - // In 3-valued logic, null = value returns UNKNOWN (null) - expect(compiled({})).toBe(null) + // eq(null, 5) means "is 5 null?" - returns false + expect(compiled({})).toBe(false) }) - it(`handles eq with value and null (3-valued logic)`, () => { + it(`handles eq with literal value and literal null (IS NULL semantics)`, () => { const func = new Func(`eq`, [new Value(5), new Value(null)]) const compiled = compileExpression(func) - // In 3-valued logic, value = null returns UNKNOWN (null) - expect(compiled({})).toBe(null) + // eq(5, null) means "is 5 null?" - returns false + expect(compiled({})).toBe(false) }) - it(`handles eq with undefined and value (3-valued logic)`, () => { + it(`handles eq with literal undefined and literal value (IS NULL semantics)`, () => { const func = new Func(`eq`, [new Value(undefined), new Value(5)]) const compiled = compileExpression(func) - // In 3-valued logic, undefined = value returns UNKNOWN (null) - expect(compiled({})).toBe(null) + // eq(undefined, 5) means "is 5 undefined?" - returns false + expect(compiled({})).toBe(false) + }) + + it(`handles eq with column value that is null and literal null (IS NULL semantics)`, () => { + const func = new Func(`eq`, [ + new PropRef([`user`, `email`]), + new Value(null), + ]) + const compiled = compileExpression(func) + + // eq(col, null) where col is null returns true + expect(compiled({ user: { email: null } })).toBe(true) + // eq(col, null) where col is undefined returns true + expect(compiled({ user: { email: undefined } })).toBe(true) + // eq(col, null) where col has a value returns false + expect(compiled({ user: { email: `test@example.com` } })).toBe( + false + ) + }) + + it(`handles eq with columns that both have null values (3-valued logic)`, () => { + // When comparing two column references (not literal nulls), + // 3-valued logic still applies + const func = new Func(`eq`, [ + new PropRef([`user`, `email`]), + new PropRef([`other`, `email`]), + ]) + const compiled = compileExpression(func) + + // When both columns are null, return UNKNOWN (null) per 3-valued logic + expect( + compiled({ user: { email: null }, other: { email: null } }) + ).toBe(null) + // When one column is null, return UNKNOWN (null) + expect( + compiled({ user: { email: null }, other: { email: `test` } }) + ).toBe(null) }) it(`handles eq with matching values`, () => { From 3f466fe513b19050bb09aff144120a26c0f1a16c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 3 Dec 2025 02:25:25 +0000 Subject: [PATCH 8/9] docs: explain eq(col, null) handling and 3-valued logic reasoning 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 --- docs/design/eq-null-handling.md | 148 ++++++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 docs/design/eq-null-handling.md diff --git a/docs/design/eq-null-handling.md b/docs/design/eq-null-handling.md new file mode 100644 index 000000000..9053c0f97 --- /dev/null +++ b/docs/design/eq-null-handling.md @@ -0,0 +1,148 @@ +# Handling `eq(col, null)` with 3-Valued Logic + +## Background + +PR #765 introduced 3-valued logic (true/false/unknown) for all comparison and logical operators to match SQL database behavior. The changeset stated: + +> With 3-valued logic, `eq(anything, null)` evaluates to `null` (UNKNOWN) and is filtered out. Use `isNull()` instead. + +This was an intentional breaking change to align local query evaluation with SQL semantics. + +## The Problem + +After PR #765, an inconsistency emerged between the SQL compiler and JavaScript evaluator when handling `eq(col, null)`: + +### SQL Compiler Behavior + +When `eq(col, null)` is compiled to SQL for Electric proxy queries, we faced a choice: + +1. Generate `"col" = $1` with `null` as the parameter → But `serialize(null)` returns empty string, causing invalid queries with missing params (the original bug) +2. Generate `"col" = NULL` → Always returns UNKNOWN in SQL, matching 3-valued logic but returning no rows +3. Transform to `"col" IS NULL` → Returns rows where col is null + +We chose option 3 because option 1 was broken and option 2 would silently return no results. + +### JavaScript Evaluator Behavior (Before Fix) + +The JS evaluator followed strict 3-valued logic: +```javascript +case `eq`: { + return (data) => { + const a = argA(data) + const b = argB(data) + if (isUnknown(a) || isUnknown(b)) { + return null // UNKNOWN + } + return a === b + } +} +``` + +### The Inconsistency + +This created a frustrating user experience: + +1. User writes: `eq(user.email, null)` +2. SQL compiler generates: `"email" IS NULL` → Data loads from Electric +3. JS evaluator returns: `null` (UNKNOWN) → `toBooleanPredicate(null)` = `false` → All rows filtered out +4. Result: Data loads but immediately disappears + +## The Solution + +We modified the JS evaluator to detect **literal null values** at compile time and transform them to `isNull` semantics: + +```javascript +case `eq`: { + const argExprA = func.args[0] + const argExprB = func.args[1] + const aIsLiteralNull = argExprA?.type === `val` && + (argExprA.value === null || argExprA.value === undefined) + const bIsLiteralNull = argExprB?.type === `val` && + (argExprB.value === null || argExprB.value === undefined) + + if (aIsLiteralNull && bIsLiteralNull) { + // eq(null, null) - always true + return () => true + } else if (aIsLiteralNull) { + // eq(null, col) - check if col is null/undefined + return (data) => argB(data) === null || argB(data) === undefined + } else if (bIsLiteralNull) { + // eq(col, null) - check if col is null/undefined + return (data) => argA(data) === null || argA(data) === undefined + } + + // Standard 3-valued logic for column-to-column comparisons + // ... +} +``` + +## Why This Approach? + +### 1. Consistency Between SQL and JS + +Both the SQL compiler and JS evaluator now handle `eq(col, null)` the same way - as an IS NULL check. Data that loads from the server won't be unexpectedly filtered out locally. + +### 2. Preserves 3-Valued Logic Where It Matters + +3-valued logic still applies for **column-to-column comparisons**: + +```typescript +// 3-valued logic applies - returns UNKNOWN if either column is null +eq(user.email, manager.email) +``` + +This is the important case for 3-valued logic - when you're comparing two columns and one might unexpectedly be null in the data. + +### 3. Handles Dynamic Values Gracefully + +If a user has a dynamic filter value that might be null: + +```typescript +const filterValue = getUserInput() // might be null +q.where(({ user }) => eq(user.name, filterValue)) +``` + +With strict 3-valued logic, this would silently return no results when `filterValue` is null. With our fix, it correctly returns rows where `name` is null. + +Without this fix, users would need to write: +```typescript +q.where(({ user }) => + filterValue === null + ? isNull(user.name) + : eq(user.name, filterValue) +) +``` + +### 4. Follows User Intent + +When a user explicitly writes `eq(col, null)`, they almost certainly mean "find rows where col is null". The strict 3-valued interpretation (return UNKNOWN, filter out all rows) is technically correct but practically useless. + +## The Distinction + +| Expression | Behavior | Rationale | +|------------|----------|-----------| +| `eq(col, null)` | Returns `true` if col is null | User explicitly asked for null comparison | +| `eq(col, variable)` where variable is null | Returns `true` if col is null | Same as above - the null is a literal value in the IR | +| `eq(col1, col2)` where col2 is null at runtime | Returns `null` (UNKNOWN) | 3-valued logic - null in data is unexpected | + +The key insight is that **literal null values** (type `val` in the IR) represent intentional null comparisons, while **null column values** at runtime represent missing/unknown data where 3-valued logic is appropriate. + +## Other Comparison Operators + +For other operators (`gt`, `lt`, `gte`, `lte`, `like`, `ilike`), comparing with null doesn't have a sensible interpretation like "IS NULL", so the SQL compiler throws an error: + +``` +Cannot use null/undefined value with 'gt' operator. Use isNull() or isUndefined() to check for null/undefined values. +``` + +This guides users toward the correct approach while allowing `eq(col, null)` to work intuitively. + +## Summary + +This is a pragmatic middle-ground that: +- Maintains SQL/JS consistency for `eq(col, null)` +- Preserves 3-valued logic for column-to-column comparisons +- Handles dynamic null values gracefully +- Follows user intent when they explicitly compare to null + +The alternative (strict 3-valued logic everywhere) would require users to always use `isNull()` and handle dynamic values explicitly, which is more error-prone and less intuitive. From 9401b42761d91c1001469119d504277fe3b32416 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 3 Dec 2025 16:18:24 +0000 Subject: [PATCH 9/9] fix: throw error for eq(col, null) - use isNull() instead 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. --- docs/design/eq-null-handling.md | 155 +++++++----------- .../src/suites/predicates.suite.ts | 87 ---------- packages/db/src/query/compiler/evaluators.ts | 32 ---- .../tests/query/compiler/evaluators.test.ts | 60 ++----- .../src/sql-compiler.ts | 22 +-- .../tests/sql-compiler.test.ts | 116 ++++++------- 6 files changed, 130 insertions(+), 342 deletions(-) diff --git a/docs/design/eq-null-handling.md b/docs/design/eq-null-handling.md index 9053c0f97..83dac8208 100644 --- a/docs/design/eq-null-handling.md +++ b/docs/design/eq-null-handling.md @@ -1,4 +1,4 @@ -# Handling `eq(col, null)` with 3-Valued Logic +# Handling Null Values in Comparison Operators ## Background @@ -10,102 +10,61 @@ This was an intentional breaking change to align local query evaluation with SQL ## The Problem -After PR #765, an inconsistency emerged between the SQL compiler and JavaScript evaluator when handling `eq(col, null)`: +When using cursor-based pagination (e.g., `useLiveInfiniteQuery`), if a cursor column has a `null` value, the next page query would generate something like `gt(col, null)`. This caused invalid SQL queries with missing parameters because `serialize(null)` returns an empty string. -### SQL Compiler Behavior - -When `eq(col, null)` is compiled to SQL for Electric proxy queries, we faced a choice: - -1. Generate `"col" = $1` with `null` as the parameter → But `serialize(null)` returns empty string, causing invalid queries with missing params (the original bug) -2. Generate `"col" = NULL` → Always returns UNKNOWN in SQL, matching 3-valued logic but returning no rows -3. Transform to `"col" IS NULL` → Returns rows where col is null - -We chose option 3 because option 1 was broken and option 2 would silently return no results. - -### JavaScript Evaluator Behavior (Before Fix) - -The JS evaluator followed strict 3-valued logic: -```javascript -case `eq`: { - return (data) => { - const a = argA(data) - const b = argB(data) - if (isUnknown(a) || isUnknown(b)) { - return null // UNKNOWN - } - return a === b - } -} +Example of the bug: +``` +subset__where="projectId" = $1 AND "name" > $2 +subset__params={"1":"uuid..."} // missing $2! ``` -### The Inconsistency +## The Solution -This created a frustrating user experience: +All comparison operators (`eq`, `gt`, `gte`, `lt`, `lte`, `like`, `ilike`) now throw a clear error when used with `null` or `undefined` values: -1. User writes: `eq(user.email, null)` -2. SQL compiler generates: `"email" IS NULL` → Data loads from Electric -3. JS evaluator returns: `null` (UNKNOWN) → `toBooleanPredicate(null)` = `false` → All rows filtered out -4. Result: Data loads but immediately disappears +``` +Cannot use null/undefined value with 'eq' operator. +Comparisons with null always evaluate to UNKNOWN in SQL. +Use isNull() or isUndefined() to check for null values, +or filter out null values before building the query. +``` -## The Solution +## Why Not Transform `eq(col, null)` to `IS NULL`? -We modified the JS evaluator to detect **literal null values** at compile time and transform them to `isNull` semantics: - -```javascript -case `eq`: { - const argExprA = func.args[0] - const argExprB = func.args[1] - const aIsLiteralNull = argExprA?.type === `val` && - (argExprA.value === null || argExprA.value === undefined) - const bIsLiteralNull = argExprB?.type === `val` && - (argExprB.value === null || argExprB.value === undefined) - - if (aIsLiteralNull && bIsLiteralNull) { - // eq(null, null) - always true - return () => true - } else if (aIsLiteralNull) { - // eq(null, col) - check if col is null/undefined - return (data) => argB(data) === null || argB(data) === undefined - } else if (bIsLiteralNull) { - // eq(col, null) - check if col is null/undefined - return (data) => argA(data) === null || argA(data) === undefined - } - - // Standard 3-valued logic for column-to-column comparisons - // ... -} -``` +We considered automatically transforming `eq(col, null)` to `IS NULL` syntax, but decided against it for these reasons: -## Why This Approach? +1. **Consistency with 3-valued logic design**: PR #765 explicitly documented that `eq(col, null)` should return UNKNOWN and users should use `isNull()` instead. -### 1. Consistency Between SQL and JS +2. **Explicitness**: Requiring users to write `isNull(col)` makes the intent clear and avoids confusion about SQL null semantics. -Both the SQL compiler and JS evaluator now handle `eq(col, null)` the same way - as an IS NULL check. Data that loads from the server won't be unexpectedly filtered out locally. +3. **Runtime null values**: If we transformed literal `null` to `IS NULL`, users might expect `eq(col, variable)` where `variable` is `null` at runtime to also work like `IS NULL`. But we can't distinguish these cases at the IR level, and silently returning no results (UNKNOWN) would be confusing. -### 2. Preserves 3-Valued Logic Where It Matters +## Correct Usage -3-valued logic still applies for **column-to-column comparisons**: +### Checking for null values ```typescript -// 3-valued logic applies - returns UNKNOWN if either column is null -eq(user.email, manager.email) -``` +// Correct - use isNull() +q.where(({ user }) => isNull(user.email)) -This is the important case for 3-valued logic - when you're comparing two columns and one might unexpectedly be null in the data. +// Correct - use isUndefined() +q.where(({ user }) => isUndefined(user.deletedAt)) -### 3. Handles Dynamic Values Gracefully +// Correct - negate with not() +q.where(({ user }) => not(isNull(user.email))) +``` -If a user has a dynamic filter value that might be null: +### Handling dynamic values that might be null ```typescript const filterValue = getUserInput() // might be null -q.where(({ user }) => eq(user.name, filterValue)) -``` -With strict 3-valued logic, this would silently return no results when `filterValue` is null. With our fix, it correctly returns rows where `name` is null. +// Option 1: Filter out null values before building the query +if (filterValue !== null) { + q.where(({ user }) => eq(user.name, filterValue)) +} -Without this fix, users would need to write: -```typescript +// Option 2: Handle null case explicitly q.where(({ user }) => filterValue === null ? isNull(user.name) @@ -113,36 +72,34 @@ q.where(({ user }) => ) ``` -### 4. Follows User Intent - -When a user explicitly writes `eq(col, null)`, they almost certainly mean "find rows where col is null". The strict 3-valued interpretation (return UNKNOWN, filter out all rows) is technically correct but practically useless. +### Cursor-based pagination -## The Distinction +The original bug was caused by cursor columns having null values. The fix is to ensure cursor columns are never null: -| Expression | Behavior | Rationale | -|------------|----------|-----------| -| `eq(col, null)` | Returns `true` if col is null | User explicitly asked for null comparison | -| `eq(col, variable)` where variable is null | Returns `true` if col is null | Same as above - the null is a literal value in the IR | -| `eq(col1, col2)` where col2 is null at runtime | Returns `null` (UNKNOWN) | 3-valued logic - null in data is unexpected | - -The key insight is that **literal null values** (type `val` in the IR) represent intentional null comparisons, while **null column values** at runtime represent missing/unknown data where 3-valued logic is appropriate. +```typescript +// Add a secondary orderBy on a non-nullable column (like primary key) +q.orderBy(({ user }) => [ + asc(user.name), // might be null + asc(user.id), // never null - ensures valid cursor +]) +``` -## Other Comparison Operators +## 3-Valued Logic Behavior -For other operators (`gt`, `lt`, `gte`, `lte`, `like`, `ilike`), comparing with null doesn't have a sensible interpretation like "IS NULL", so the SQL compiler throws an error: +With the current implementation, 3-valued logic applies consistently: -``` -Cannot use null/undefined value with 'gt' operator. Use isNull() or isUndefined() to check for null/undefined values. -``` +| Expression | Result | +|------------|--------| +| `eq(col, null)` | **Error** - use `isNull(col)` | +| `eq(col, value)` where `col` is null at runtime | `null` (UNKNOWN) | +| `eq(col1, col2)` where either is null at runtime | `null` (UNKNOWN) | +| `isNull(col)` | `true` or `false` | -This guides users toward the correct approach while allowing `eq(col, null)` to work intuitively. +The UNKNOWN result from 3-valued logic causes rows to be filtered out (not included in WHERE clause results), matching SQL behavior. ## Summary -This is a pragmatic middle-ground that: -- Maintains SQL/JS consistency for `eq(col, null)` -- Preserves 3-valued logic for column-to-column comparisons -- Handles dynamic null values gracefully -- Follows user intent when they explicitly compare to null - -The alternative (strict 3-valued logic everywhere) would require users to always use `isNull()` and handle dynamic values explicitly, which is more error-prone and less intuitive. +- All comparison operators throw errors for null/undefined values +- Use `isNull()` or `isUndefined()` to check for null values +- 3-valued logic applies when column values are null at runtime +- Cursor-based pagination should use non-nullable columns or add a secondary sort key diff --git a/packages/db-collection-e2e/src/suites/predicates.suite.ts b/packages/db-collection-e2e/src/suites/predicates.suite.ts index ac572114c..a4f446856 100644 --- a/packages/db-collection-e2e/src/suites/predicates.suite.ts +++ b/packages/db-collection-e2e/src/suites/predicates.suite.ts @@ -702,93 +702,6 @@ export function createPredicatesTestSuite( await query.cleanup() }) - it(`should filter with eq(col, null) same as isNull()`, async () => { - // eq(col, null) should be transformed to IS NULL and work correctly - const config = await getConfig() - const usersCollection = config.collections.onDemand.users - - // First verify the existing isNull test pattern works - const isNullQuery = createLiveQueryCollection((q) => - q - .from({ user: usersCollection }) - .where(({ user }) => isNull(user.email)) - ) - await isNullQuery.preload() - await waitForQueryData(isNullQuery, { minSize: 1, timeout: 5000 }) - const isNullResults = Array.from(isNullQuery.state.values()) - await isNullQuery.cleanup() - - // Now test eq(col, null) produces same results - const query = createLiveQueryCollection((q) => - q - .from({ user: usersCollection }) - .where(({ user }) => eq(user.email, null)) - ) - - await query.preload() - await waitForQueryData(query, { minSize: 1, timeout: 5000 }) - - const results = Array.from(query.state.values()) - expect(results.length).toBeGreaterThan(0) - // Should return same count as isNull query - expect(results.length).toBe(isNullResults.length) - assertAllItemsMatch(query, (u) => u.email === null) - - await query.cleanup() - }) - - it(`should filter with eq(col, null) in AND clause`, async () => { - // Tests that eq(col, null) works correctly when combined with other predicates - const config = await getConfig() - const usersCollection = config.collections.onDemand.users - - const query = createLiveQueryCollection((q) => - q - .from({ user: usersCollection }) - .where(({ user }) => - and(eq(user.email, null), eq(user.isActive, true)) - ) - ) - - await query.preload() - await waitForQueryData(query, { minSize: 1, timeout: 5000 }) - - const results = Array.from(query.state.values()) - expect(results.length).toBeGreaterThan(0) - assertAllItemsMatch( - query, - (u) => u.email === null && u.isActive === true - ) - - await query.cleanup() - }) - - it(`should filter with eq(col, null) in OR clause`, async () => { - // Tests that eq(col, null) works correctly in OR with other predicates - const config = await getConfig() - const usersCollection = config.collections.onDemand.users - - const query = createLiveQueryCollection((q) => - q - .from({ user: usersCollection }) - .where(({ user }) => - or(eq(user.email, null), eq(user.name, `Alice 0`)) - ) - ) - - await query.preload() - await waitForQueryData(query, { minSize: 1 }) - - const results = Array.from(query.state.values()) - expect(results.length).toBeGreaterThan(0) - assertAllItemsMatch( - query, - (u) => u.email === null || u.name === `Alice 0` - ) - - await query.cleanup() - }) - it(`should filter with not(isNull()) on nullable field`, async () => { const config = await getConfig() const usersCollection = config.collections.onDemand.users diff --git a/packages/db/src/query/compiler/evaluators.ts b/packages/db/src/query/compiler/evaluators.ts index 1b1476a5c..19c69663d 100644 --- a/packages/db/src/query/compiler/evaluators.ts +++ b/packages/db/src/query/compiler/evaluators.ts @@ -163,38 +163,6 @@ function compileFunction(func: Func, isSingleRow: boolean): (data: any) => any { switch (func.name) { // Comparison operators case `eq`: { - // Check at compile time if one argument is a literal null/undefined value - // If so, transform to isNull semantics (return true/false, not UNKNOWN) - // This matches SQL behavior where eq(col, null) is transformed to IS NULL - const argExprA = func.args[0] - const argExprB = func.args[1] - const aIsLiteralNull = - argExprA?.type === `val` && - (argExprA.value === null || argExprA.value === undefined) - const bIsLiteralNull = - argExprB?.type === `val` && - (argExprB.value === null || argExprB.value === undefined) - - if (aIsLiteralNull && bIsLiteralNull) { - // eq(null, null) or eq(undefined, undefined) - always true - return () => true - } else if (aIsLiteralNull) { - // eq(null, col) - check if col value is null/undefined - const argB = compiledArgs[1]! - return (data) => { - const b = argB(data) - return b === null || b === undefined - } - } else if (bIsLiteralNull) { - // eq(col, null) - check if col value is null/undefined - const argA = compiledArgs[0]! - return (data) => { - const a = argA(data) - return a === null || a === undefined - } - } - - // Standard equality comparison (no literal nulls) const argA = compiledArgs[0]! const argB = compiledArgs[1]! return (data) => { diff --git a/packages/db/tests/query/compiler/evaluators.test.ts b/packages/db/tests/query/compiler/evaluators.test.ts index a9412c3f7..64a867196 100644 --- a/packages/db/tests/query/compiler/evaluators.test.ts +++ b/packages/db/tests/query/compiler/evaluators.test.ts @@ -378,72 +378,36 @@ describe(`evaluators`, () => { describe(`comparison operators`, () => { describe(`eq (equality)`, () => { - it(`handles eq with literal null and literal null (IS NULL semantics)`, () => { + it(`handles eq with null and null (3-valued logic)`, () => { const func = new Func(`eq`, [new Value(null), new Value(null)]) const compiled = compileExpression(func) - // eq(null, null) returns true - both nulls are equal in IS NULL semantics - expect(compiled({})).toBe(true) + // In 3-valued logic, null = null returns UNKNOWN (null) + expect(compiled({})).toBe(null) }) - it(`handles eq with literal null and literal value (IS NULL semantics)`, () => { + it(`handles eq with null and value (3-valued logic)`, () => { const func = new Func(`eq`, [new Value(null), new Value(5)]) const compiled = compileExpression(func) - // eq(null, 5) means "is 5 null?" - returns false - expect(compiled({})).toBe(false) + // In 3-valued logic, null = value returns UNKNOWN (null) + expect(compiled({})).toBe(null) }) - it(`handles eq with literal value and literal null (IS NULL semantics)`, () => { + it(`handles eq with value and null (3-valued logic)`, () => { const func = new Func(`eq`, [new Value(5), new Value(null)]) const compiled = compileExpression(func) - // eq(5, null) means "is 5 null?" - returns false - expect(compiled({})).toBe(false) + // In 3-valued logic, value = null returns UNKNOWN (null) + expect(compiled({})).toBe(null) }) - it(`handles eq with literal undefined and literal value (IS NULL semantics)`, () => { + it(`handles eq with undefined and value (3-valued logic)`, () => { const func = new Func(`eq`, [new Value(undefined), new Value(5)]) const compiled = compileExpression(func) - // eq(undefined, 5) means "is 5 undefined?" - returns false - expect(compiled({})).toBe(false) - }) - - it(`handles eq with column value that is null and literal null (IS NULL semantics)`, () => { - const func = new Func(`eq`, [ - new PropRef([`user`, `email`]), - new Value(null), - ]) - const compiled = compileExpression(func) - - // eq(col, null) where col is null returns true - expect(compiled({ user: { email: null } })).toBe(true) - // eq(col, null) where col is undefined returns true - expect(compiled({ user: { email: undefined } })).toBe(true) - // eq(col, null) where col has a value returns false - expect(compiled({ user: { email: `test@example.com` } })).toBe( - false - ) - }) - - it(`handles eq with columns that both have null values (3-valued logic)`, () => { - // When comparing two column references (not literal nulls), - // 3-valued logic still applies - const func = new Func(`eq`, [ - new PropRef([`user`, `email`]), - new PropRef([`other`, `email`]), - ]) - const compiled = compileExpression(func) - - // When both columns are null, return UNKNOWN (null) per 3-valued logic - expect( - compiled({ user: { email: null }, other: { email: null } }) - ).toBe(null) - // When one column is null, return UNKNOWN (null) - expect( - compiled({ user: { email: null }, other: { email: `test` } }) - ).toBe(null) + // In 3-valued logic, undefined = value returns UNKNOWN (null) + expect(compiled({})).toBe(null) }) it(`handles eq with matching values`, () => { diff --git a/packages/electric-db-collection/src/sql-compiler.ts b/packages/electric-db-collection/src/sql-compiler.ts index 8ad9c4700..7e4289a04 100644 --- a/packages/electric-db-collection/src/sql-compiler.ts +++ b/packages/electric-db-collection/src/sql-compiler.ts @@ -141,31 +141,15 @@ function compileFunction( // Handle comparison operators with null/undefined values // These would create invalid queries with missing params (e.g., "col = $1" with empty params) + // In SQL, all comparisons with NULL return UNKNOWN, so these are almost always mistakes if (isComparisonOp(name)) { const nullArgIndex = args.findIndex((arg: IR.BasicExpression) => isNullValue(arg) ) if (nullArgIndex !== -1) { - const otherArgIndex = nullArgIndex === 0 ? 1 : 0 - const otherArg = args[otherArgIndex] - - if ( - name === `eq` && - otherArg && - !isNullValue(otherArg) && - (otherArg.type === `ref` || otherArg.type === `func`) - ) { - // Transform eq(col, null/undefined) or eq(func(...), null/undefined) to "expr IS NULL" - // This is the semantically correct behavior in SQL - // We only allow ref and func types - comparing a literal to null (e.g., eq(42, null)) - // is almost certainly a mistake and should throw - return `${compileBasicExpression(otherArg, params)} IS NULL` - } - - // For other comparison operators (gt, lt, gte, lte, like, ilike), - // or eq where both args are null/undefined, - // null comparisons don't make semantic sense in SQL (they always evaluate to UNKNOWN) + // All comparison operators (including eq) throw an error for null values + // Users should use isNull() or isUndefined() to check for null values throw new Error( `Cannot use null/undefined value with '${name}' operator. ` + `Comparisons with null always evaluate to UNKNOWN in SQL. ` + diff --git a/packages/electric-db-collection/tests/sql-compiler.test.ts b/packages/electric-db-collection/tests/sql-compiler.test.ts index 8b956f8fd..6d6dc9a2a 100644 --- a/packages/electric-db-collection/tests/sql-compiler.test.ts +++ b/packages/electric-db-collection/tests/sql-compiler.test.ts @@ -109,28 +109,29 @@ describe(`sql-compiler`, () => { }) describe(`null/undefined value handling`, () => { - it(`should transform eq(col, null) to IS NULL`, () => { - const result = compileSQL({ - where: func(`eq`, [ref(`deletedAt`), val(null)]), - }) - expect(result.where).toBe(`"deletedAt" IS NULL`) - expect(result.params).toEqual({}) + it(`should throw error for eq(col, null)`, () => { + // Users should use isNull() instead of eq(col, null) + expect(() => + compileSQL({ + where: func(`eq`, [ref(`deletedAt`), val(null)]), + }) + ).toThrow(`Cannot use null/undefined value with 'eq' operator`) }) - it(`should transform eq(col, undefined) to IS NULL`, () => { - const result = compileSQL({ - where: func(`eq`, [ref(`deletedAt`), val(undefined)]), - }) - expect(result.where).toBe(`"deletedAt" IS NULL`) - expect(result.params).toEqual({}) + it(`should throw error for eq(col, undefined)`, () => { + expect(() => + compileSQL({ + where: func(`eq`, [ref(`deletedAt`), val(undefined)]), + }) + ).toThrow(`Cannot use null/undefined value with 'eq' operator`) }) - it(`should transform eq(null, col) to IS NULL (reversed order)`, () => { - const result = compileSQL({ - where: func(`eq`, [val(null), ref(`name`)]), - }) - expect(result.where).toBe(`"name" IS NULL`) - expect(result.params).toEqual({}) + it(`should throw error for eq(null, col) (reversed order)`, () => { + expect(() => + compileSQL({ + where: func(`eq`, [val(null), ref(`name`)]), + }) + ).toThrow(`Cannot use null/undefined value with 'eq' operator`) }) it(`should throw error for gt with null value`, () => { @@ -181,29 +182,27 @@ describe(`sql-compiler`, () => { ).toThrow(`Cannot use null/undefined value with 'ilike' operator`) }) - it(`should handle eq(col, null) in AND clause with two conditions`, () => { - const result = compileSQL({ - where: func(`and`, [ - func(`eq`, [ref(`projectId`), val(`uuid-123`)]), - func(`eq`, [ref(`deletedAt`), val(null)]), - ]), - }) - expect(result.where).toBe(`"projectId" = $1 AND "deletedAt" IS NULL`) - expect(result.params).toEqual({ "1": `uuid-123` }) + it(`should throw error for eq(col, null) in AND clause`, () => { + expect(() => + compileSQL({ + where: func(`and`, [ + func(`eq`, [ref(`projectId`), val(`uuid-123`)]), + func(`eq`, [ref(`deletedAt`), val(null)]), + ]), + }) + ).toThrow(`Cannot use null/undefined value with 'eq' operator`) }) - it(`should handle mixed null and value conditions in AND with >2 conditions`, () => { - const result = compileSQL({ - where: func(`and`, [ - func(`eq`, [ref(`status`), val(`active`)]), - func(`eq`, [ref(`archivedAt`), val(null)]), - func(`gt`, [ref(`createdAt`), val(`2024-01-01`)]), - ]), - }) - expect(result.where).toBe( - `("status" = $1) AND ("archivedAt" IS NULL) AND ("createdAt" > $2)` - ) - expect(result.params).toEqual({ "1": `active`, "2": `2024-01-01` }) + it(`should throw error for eq(col, null) in mixed conditions`, () => { + expect(() => + compileSQL({ + where: func(`and`, [ + func(`eq`, [ref(`status`), val(`active`)]), + func(`eq`, [ref(`archivedAt`), val(null)]), + func(`gt`, [ref(`createdAt`), val(`2024-01-01`)]), + ]), + }) + ).toThrow(`Cannot use null/undefined value with 'eq' operator`) }) it(`should not include params for null values in complex queries`, () => { @@ -240,29 +239,32 @@ describe(`sql-compiler`, () => { ).toThrow(`Cannot use null/undefined value with 'eq' operator`) }) - it(`should produce identical output for eq(col, null) and isNull(col)`, () => { - // Ensure eq(col, null) produces exactly the same SQL as isNull(col) - const eqResult = compileSQL({ - where: func(`eq`, [ref(`email`), val(null)]), - }) + it(`should throw error for eq(col, null) - use isNull(col) instead`, () => { + // eq(col, null) should throw an error + // Users should use isNull(col) which works correctly + expect(() => + compileSQL({ + where: func(`eq`, [ref(`email`), val(null)]), + }) + ).toThrow(`Cannot use null/undefined value with 'eq' operator`) + + // isNull(col) should work correctly const isNullResult = compileSQL({ where: func(`isNull`, [ref(`email`)]), }) - expect(eqResult.where).toBe(isNullResult.where) - expect(eqResult.params).toEqual(isNullResult.params) - expect(eqResult.where).toBe(`"email" IS NULL`) - expect(eqResult.params).toEqual({}) + expect(isNullResult.where).toBe(`"email" IS NULL`) + expect(isNullResult.params).toEqual({}) }) - it(`should handle eq(col, null) in OR clause`, () => { - const result = compileSQL({ - where: func(`or`, [ - func(`eq`, [ref(`deletedAt`), val(null)]), - func(`eq`, [ref(`status`), val(`active`)]), - ]), - }) - expect(result.where).toBe(`"deletedAt" IS NULL OR "status" = $1`) - expect(result.params).toEqual({ "1": `active` }) + it(`should throw error for eq(col, null) in OR clause`, () => { + expect(() => + compileSQL({ + where: func(`or`, [ + func(`eq`, [ref(`deletedAt`), val(null)]), + func(`eq`, [ref(`status`), val(`active`)]), + ]), + }) + ).toThrow(`Cannot use null/undefined value with 'eq' operator`) }) })