From b37dfc7ec15c5f9655ea08f54f3f849082a64e06 Mon Sep 17 00:00:00 2001 From: Dylan Frankland Date: Thu, 2 Jul 2026 18:17:12 +0000 Subject: [PATCH 1/2] Bind null query params as SQL NULL (\N), matching @clickhouse/client formatParamValue rejected a top-level null/undefined with ChdbBindError. The reference client (@clickhouse/client-common formatQueryParams) instead emits the TSV null marker \N at the top level (and the NULL keyword when nested), which the server binds as SQL NULL for Nullable columns. chdb-node is a byte-compat @clickhouse/client facade (#49, #52), so throwing was a divergence, not an engine limitation: the native chdb_query_with_params path already accepts \N as NULL (verified for Nullable(String)/Int64). Emit \N for a top-level null/undefined; nested nulls already render as the NULL keyword via serializeValue. A null bound to a non-Nullable placeholder is now rejected by the engine (a CHDB_* query error) rather than a client-side ChdbBindError, matching the HTTP client + server. Removes the two NULL-binding entries from the clickhouse-js parity skip list; both now pass against @clickhouse/client 1.23.0. Fixes #67 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/serialize.ts | 19 ++++++++++------ test/v3/querybind.test.ts | 36 +++++++++++++++++++++++++++--- test/v3/serialize.test.ts | 32 +++++++++++++++++++++++++- tests/clickhouse-js/skip_list.json | 10 --------- 4 files changed, 76 insertions(+), 21 deletions(-) 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/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", From caf4f69b0fae702bfa828fa6494ad05dadc14115 Mon Sep 17 00:00:00 2001 From: Dylan Frankland Date: Mon, 6 Jul 2026 20:58:22 +0000 Subject: [PATCH 2/2] test: undefined binds as \N NULL, not a bind error The null-param compat fix maps top-level null/undefined to the TSV null marker \N (matching @clickhouse/client). Binding NULL to a non-Nullable {x:UInt32} placeholder is rejected by the engine (CHDB_QUERY), not at serialization (CHDB_BIND). Drop undefined from the unserializable-values case and add a dedicated null/undefined -> \N assertion. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/v3/robustness.test.ts | Bin 4608 -> 5133 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/test/v3/robustness.test.ts b/test/v3/robustness.test.ts index af4742d4ddd704e78ba91838ba6472fad3365068..892d35e2594be036aa102b4b1ac0755ba7410047 100644 GIT binary patch delta 424 zcmZ8d!AiqG6hshGdlCO&4nk8%8>k0Cv8Yg_Py*7{ixrwCug%(Zx9)DlTD0QbQ@%qG zy!b2rjhh5LbWgiG^XARW>;Aj@`Qc>W&d++nM64lF&iz!xG-ZNfgc|+J9)e-72X{0I z>c=24vk(;BQe-rk!Bi@Q5K=S(I~`6q!7LY4^WfHT(#EO^dwd@QxoB{DAilXqlUlQo zvnM+#VG_cie^roh7pgfaj7P!Pvkwy`Ow9fSPb1Evc_LFyzP*w#O`O{tmQs4QY8%d0 zk4M90*5+xabC*nno0Io_i|eqS3Z`_bI6*NGYj28{60~ z8=&wX8YV>vH4Bq@H*O?_Hboq!6UZs9^O`;U_k2@Fwzb{L8kIdKhb%2Ss~l##j`Q_W HdE5B~1Nn_4 delta 28 kcmeCxXi(X(mYusaFC{fCGcPqoM`7|>_M4mMapiLY0Glxix&QzG