diff --git a/.changeset/fix-electric-null-params.md b/.changeset/fix-electric-null-params.md new file mode 100644 index 000000000..9effab7e3 --- /dev/null +++ b/.changeset/fix-electric-null-params.md @@ -0,0 +1,9 @@ +--- +'@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 all comparison operators (eq, gt, lt, gte, lte, like, ilike) throw a clear error when used with null/undefined values, since comparisons with NULL always evaluate to UNKNOWN in SQL. Users should use `isNull()` or `isUndefined()` to check for null values instead. diff --git a/packages/electric-db-collection/src/sql-compiler.ts b/packages/electric-db-collection/src/sql-compiler.ts index 6c1db6c4c..d7d021c89 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,26 @@ 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) + // 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) { + // 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. ` + + `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), ) @@ -176,7 +203,7 @@ function compileFunction( // Special case for comparison operators with boolean values // PostgreSQL doesn't support < > <= >= on booleans // Transform to equivalent equality checks or constant expressions - if (isComparisonOp(name)) { + if (isBooleanComparisonOp(name)) { const lhsArg = args[0] const rhsArg = args[1] @@ -312,11 +339,21 @@ 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) +} + /** * Checks if the operator is a comparison operator (excluding eq) * These operators don't work on booleans in PostgreSQL without casting */ -function isComparisonOp(name: string): boolean { +function isBooleanComparisonOp(name: string): boolean { return [`gt`, `gte`, `lt`, `lte`].includes(name) } 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..9989d47f9 --- /dev/null +++ b/packages/electric-db-collection/tests/sql-compiler.test.ts @@ -0,0 +1,312 @@ +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): IR.BasicExpression { + return { type: `val`, value } as IR.BasicExpression +} + +// Helper to create a reference expression +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): IR.BasicExpression { + return { type: `func`, name, args } as IR.BasicExpression +} + +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 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 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 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`, () => { + 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 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 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`, () => { + // 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`) + }) + + 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 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(isNullResult.where).toBe(`"email" IS NULL`) + expect(isNullResult.params).toEqual({}) + }) + + 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`) + }) + }) + + 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) + }) + }) + }) +})