diff --git a/src/serialize.ts b/src/serialize.ts index 0908459..3bb0d00 100644 --- a/src/serialize.ts +++ b/src/serialize.ts @@ -218,15 +218,20 @@ function serializeArray(arr: ReadonlyArray): string { * Because the engine binds the value (never interpolates it into SQL), there is * no escaping/injection surface here at all. * - * @throws ChdbBindError on null/undefined (use a typed NULL in SQL instead), - * non-finite or unsafe-integer numbers, invalid Dates, or unsupported types. + * A top-level `null`/`undefined` becomes the TSV null marker `\N`, byte-for-byte + * matching @clickhouse/client-common's `formatQueryParams` (which emits `\N` + * when `printNullAsKeyword` is false — the default at the top level). The engine + * binds `\N` as SQL NULL for a Nullable placeholder (verified against + * Nullable(String) and Nullable(Int64)); binding it to a non-Nullable + * placeholder is rejected by the ENGINE, exactly as the HTTP client + server + * would. A `null` NESTED inside an Array/Map keeps the `NULL` keyword form (see + * serializeValue) — again matching the reference client. + * + * @throws ChdbBindError on non-finite or unsafe-integer numbers, invalid Dates, + * or unsupported types. */ export function formatParamValue(value: unknown): string { - if (value === null || value === undefined) { - throw new ChdbBindError( - 'null/undefined parameter values are not supported; omit the parameter or use a typed NULL in SQL', - ) - } + if (value === null || value === undefined) return '\\N' if (typeof value === 'string') return tsvEscape(value) // TSV/Escaped — engine binds as declared type if (value instanceof Date) return formatDateTimeUTC(value) // 'YYYY-MM-DD HH:MM:SS' (no TSV-special chars) // numbers / bigint / boolean / Array / TypedArray / Map / object: the diff --git a/test/v3/querybind.test.ts b/test/v3/querybind.test.ts index cf152be..0da88f0 100644 --- a/test/v3/querybind.test.ts +++ b/test/v3/querybind.test.ts @@ -37,7 +37,7 @@ describe('queryBind path A (server-side chdb_query_with_params)', () => { } }) - it('rejects unsafe-integer / null params with a typed ChdbBindError', () => { + it('rejects unsafe-integer params with a typed ChdbBindError', () => { try { queryBind('SELECT {n:Int64}', { n: 1e21 }, 'CSV') expect.unreachable('expected ChdbBindError') @@ -45,11 +45,41 @@ describe('queryBind path A (server-side chdb_query_with_params)', () => { expect(e.name).toBe('ChdbBindError') expect(e.code).toBe('CHDB_BIND') } + }) + + it('binds a null param as SQL NULL against a Nullable placeholder (\\N, @clickhouse/client-compatible)', () => { + // null / undefined -> the TSV null marker \N, which the engine binds as NULL. + for (const nullish of [null, undefined]) { + const row = JSON.parse( + queryBind( + 'SELECT {v:Nullable(String)} AS v, ({v:Nullable(String)} IS NULL) AS isNull', + { v: nullish }, + 'JSONEachRow', + ).trim(), + ) + expect(row).toEqual({ v: null, isNull: 1 }) + } + // Non-string Nullable columns bind \N as NULL too. + expect(t('SELECT {n:Nullable(Int64)} IS NULL', { n: null })).toBe('1') + // The literal string "NULL" is NOT coerced to null — only \N is the marker. + expect(t('SELECT {v:Nullable(String)} IS NULL', { v: 'NULL' })).toBe('0') + // A null NESTED in an Array uses the NULL keyword and round-trips element-wise. + const arr = JSON.parse( + queryBind('SELECT {xs:Array(Nullable(Int64))} AS xs', { xs: [1, null, 3] }, 'JSONEachRow').trim(), + ) + expect(arr.xs).toEqual([1, null, 3]) + }) + + it('lets the engine reject a null bound to a non-Nullable placeholder', () => { + // Matches the HTTP client + server: \N against a non-Nullable type is an + // ENGINE error, not a client-side bind throw. try { queryBind('SELECT {n:Int64}', { n: null }, 'CSV') - expect.unreachable('expected ChdbBindError') + expect.unreachable('expected an engine error') } catch (e: any) { - expect(e.name).toBe('ChdbBindError') + expect(e).toBeInstanceOf(Error) + expect(String(e.code)).toMatch(/^CHDB_/) + expect(e.name).not.toBe('ChdbBindError') } }) diff --git a/test/v3/robustness.test.ts b/test/v3/robustness.test.ts index af4742d..892d35e 100644 Binary files a/test/v3/robustness.test.ts and b/test/v3/robustness.test.ts differ diff --git a/test/v3/serialize.test.ts b/test/v3/serialize.test.ts index 6aaec2e..e55e972 100644 --- a/test/v3/serialize.test.ts +++ b/test/v3/serialize.test.ts @@ -1,5 +1,10 @@ import { describe, it, expect } from 'vitest' -import { escapeStringLiteral, validateIdentifier, serializeValue } from '../../src/serialize' +import { + escapeStringLiteral, + validateIdentifier, + serializeValue, + formatParamValue, +} from '../../src/serialize' import { ChdbBindError } from '../../src/errors' // Round-trip literals through the REAL engine to prove correctness + injection safety. import { query } from '../../index.js' @@ -96,6 +101,31 @@ describe('serializeValue (string assertions)', () => { }) }) +describe('formatParamValue (server-side {name:Type} binding)', () => { + it('emits the TSV null marker \\N for a top-level null/undefined', () => { + // Byte-for-byte matches @clickhouse/client-common formatQueryParams + // (printNullAsKeyword=false at the top level) — was a ChdbBindError throw. + expect(formatParamValue(null)).toBe('\\N') + expect(formatParamValue(undefined)).toBe('\\N') + }) + + it('keeps the NULL keyword for a null nested inside an Array/Map', () => { + expect(formatParamValue([1, null, 3])).toBe('[1,NULL,3]') + expect(formatParamValue(new Map([['k', null]]))).toBe("{'k':NULL}") + }) + + it('TSV-escapes a top-level string without SQL-quoting it', () => { + expect(formatParamValue('a\tb')).toBe('a\\tb') + expect(formatParamValue("no'quote")).toBe("no'quote") + }) + + it('still rejects non-finite / unsafe / unserializable values', () => { + expect(() => formatParamValue(NaN)).toThrow(ChdbBindError) + expect(() => formatParamValue(1e21)).toThrow(ChdbBindError) + expect(() => formatParamValue(Symbol('x'))).toThrow(ChdbBindError) + }) +}) + describe('serializeValue (real-engine round-trips)', () => { it('round-trips scalar literals exactly', () => { expect(tsv(`SELECT ${serializeValue(true)}`)).toBe('true') diff --git a/tests/clickhouse-js/skip_list.json b/tests/clickhouse-js/skip_list.json index d8eaec0..c7c0124 100644 --- a/tests/clickhouse-js/skip_list.json +++ b/tests/clickhouse-js/skip_list.json @@ -252,16 +252,6 @@ "test": "select with query binding > handles maps with nullable values in a parametrized query", "reason": "chdb parameter-binding for Map(K, Nullable(V)) generates a serialization the engine parser rejects." }, - { - "file": "packages/client-common/__tests__/integration/select_query_binding.test.ts", - "test": "select with query binding > NULL parameter binding > should work with nulls", - "reason": "chdb's `queryBindAsync` NULL substitution diverges from the server's `param_*` URL-binding semantics (server-side strict-NULL handling)." - }, - { - "file": "packages/client-common/__tests__/integration/select_query_binding.test.ts", - "test": "select with query binding > NULL parameter binding > should with an explicit undefined", - "reason": "Same chdb NULL/undefined parameter-binding divergence." - }, { "file": "packages/client-common/__tests__/integration/select_query_binding.test.ts", "test": "select with query binding > Nested boolean types > handles boolean in a tuple",