fix: back-quote insert() column identifiers with special characters

[polyglotAI-bot]

Description

Fixes #945.

getInsertQuery built the INSERT INTO t (<cols>) FORMAT ... statement by joining the columns array (and the columns.except list) verbatim. A column name containing a space or other special character therefore produced invalid SQL — e.g. columns: ['test id', 'name'] generated INSERT INTO t (test id, name) FORMAT ..., which the server rejects with Code: 62 ... Syntax error.

The fix back-quotes each column identifier at the generation site, escaping backslashes and backticks exactly the way the ClickHouse server's own backQuote does (\ → \\, ` → \`). Identifiers that are already quoted (with backticks or double quotes) are passed through unchanged, so callers that pre-quoted their column names as a workaround keep working — the change is backward compatible.

Verified against ClickHouse 26.5:

• (\test id`, `name`)` is accepted; the unquoted form is the reported syntax error.
• A backslash inside a back-quoted identifier is an escape character, so it must itself be escaped (naive `a\b` targets the wrong column) — hence matching backQuote.
• Whole-string quoting a dotted/Nested column (`n.arr1`) targets the same physical column as the unquoted form, so no dot-splitting is needed.

The changed code lives in the shared common module that is bundled into both @clickhouse/client and @clickhouse/client-web, so the fix (and changelog entries) apply to both.

Changes

• packages/client-common/src/client.ts: add an escapeColumn helper and apply it to both the array and columns.except branches of getInsertQuery; update the InsertParams.columns JSDoc to document the auto-quoting + passthrough behavior.
• packages/client-node/CHANGELOG.md and packages/client-web/CHANGELOG.md: add a # 1.23.2 → ## Bug Fixes entry.

Test

• Unit (packages/client-common/__tests__/unit/insert_query_columns.test.ts, new): asserts the exact generated SQL through the real client.insert() path (mocked connection) for plain, spaced, embedded-backtick, embedded-backslash, combined backslash+backtick, lone-backtick, and dotted/Nested identifiers, plus the columns.except cases. Includes contrast cases proving already back-quoted / double-quoted identifiers are passed through unchanged.
• Integration (insert_specific_columns.test.ts): inserts into a space-named column via columns and excludes a space-named column via columns.except, then reads the data back.
• Both were confirmed to fail without the fix (server syntax error / wrong SQL) and pass with it; the existing insert tests and the full node unit suite remain green.

Pre-PR validation gate

• Deterministic repro confirmed (fails on main, passes on this branch)
• Root cause documented above
• Fix targets the root cause (back-quoting at the query-generation site)
• Test fails without fix, passes with fix
• No existing tests broken (full node unit suite + insert integration tests green)
• Convention compliance verified per AGENTS.md (changelog in both affected packages, prettier/eslint/typecheck clean)
• Backward compatible: already-quoted identifiers pass through unchanged

Note

Related source bug in another client: ClickHouse/clickhouse-cpp#30 (identifier quoting in generated INSERT column lists is an SDK-level concern each client implements independently). No backport is requested here; maintainers own any backport decision.

[Copilot]

Pull request overview

Fixes invalid SQL generation in client.insert() when columns / columns.except include identifiers that require quoting (spaces, special characters) by auto back-quoting column identifiers while preserving backward compatibility for already-quoted inputs.

Changes:

• Add an escapeColumn helper and apply it to both columns: string[] and columns: { except: string[] } paths in getInsertQuery.
• Update InsertParams.columns JSDoc to document the new quoting + passthrough behavior.
• Add unit + integration tests covering spaced identifiers and escaping behavior; add changelog entries for both Node and Web packages.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
packages/client-web/CHANGELOG.md	Adds a bug-fix entry for auto-quoting insert.columns / columns.except.
packages/client-node/CHANGELOG.md	Adds a bug-fix entry for auto-quoting insert.columns / columns.except.
packages/client-common/src/client.ts	Quotes/escapes user-provided column identifiers in generated INSERT column lists.
packages/client-common/tests/unit/insert_query_columns.test.ts	New unit coverage asserting exact generated INSERT SQL for tricky identifiers.
packages/client-common/tests/integration/insert_specific_columns.test.ts	New integration coverage for spaced column names in columns and columns.except.

[polyglotAI-bot]

Thanks for flagging this — I checked it against the repo's changelog convention and current main, and I've deliberately kept the # 1.23.2 heading. Details:

This repo adds the next-version CHANGELOG heading ahead of the package.json bump. Per AGENTS.md: entries go under the top-most heading, and "if the most recent # x.y.z heading corresponds to an already-released version (check git tag), open a new top-level # x.y.z heading." # 1.23.1 is already released — git tag has client-web-1.23.1, and npm dist-tags.latest is 1.23.1 — so a new # 1.23.2 heading is the correct choice. The package.json bump is done later in a dedicated chore: bump version release PR (that's exactly how 1.23.1 landed: #935 added the # 1.23.1 heading while package.json was still 1.23.0, and #939/#940 bumped package.json to 1.23.1 afterward).

Most relevantly: #947 was merged to main earlier today and introduces the # 1.23.2 heading itself, with package.json still at 1.23.1 — precisely this pattern. This branch now merges main, so the # 1.23.2 heading actually comes from #947 and this PR simply appends its bullet underneath; the heading matches merged-main state.

So neither suggested change applies here: bumping package.json inside a bug-fix PR would bypass the dedicated release-PR flow, and the entry can't move under # 1.23.1 because that version is already published. Happy to adjust if the maintainers prefer a different approach.

[polyglotAI-bot]

Thanks for flagging this — I checked it against the repo's changelog convention and current main, and I've deliberately kept the # 1.23.2 heading. Details:

This repo adds the next-version CHANGELOG heading ahead of the package.json bump. Per AGENTS.md: entries go under the top-most heading, and "if the most recent # x.y.z heading corresponds to an already-released version (check git tag), open a new top-level # x.y.z heading." # 1.23.1 is already released — git tag has client-1.23.1, and npm dist-tags.latest is 1.23.1 — so a new # 1.23.2 heading is the correct choice. The package.json bump is done later in a dedicated chore: bump version release PR (that's exactly how 1.23.1 landed: #935 added the # 1.23.1 heading while package.json was still 1.23.0, and #939/#940 bumped package.json to 1.23.1 afterward).

Most relevantly: #947 was merged to main earlier today and introduces the # 1.23.2 heading itself, with package.json still at 1.23.1 — precisely this pattern. This branch now merges main, so the # 1.23.2 heading actually comes from #947 and this PR simply appends its bullet underneath; the heading matches merged-main state.

So neither suggested change applies here: bumping package.json inside a bug-fix PR would bypass the dedicated release-PR flow, and the entry can't move under # 1.23.1 because that version is already published. Happy to adjust if the maintainers prefer a different approach.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[polyglotAI-bot]

CI note — the failing code-quality (Prettier) check is pre-existing, unrelated to this PR, and non-blocking.

The single failing check is the whole-tree prettier --check . step in code-quality, and it flags exactly one file:

[warn] CODE_OF_CONDUCT.md
[warn] Code style issues found in the above file. Run Prettier with --write to fix.

That file is not part of this PR's diff — it is byte-identical to main. All five files this PR actually changes (packages/client-common/src/client.ts, packages/client-node/CHANGELOG.md, packages/client-web/CHANGELOG.md, and the two new test files) are Prettier-clean.

Root cause: CODE_OF_CONDUCT.md was added to main in #942 in a state Prettier reformats. The node workflow (.github/workflows/tests-node.yml, which hosts the whole-tree code-quality job) only triggers on changes under packages/**, tests/**, package.json, etc. — root-level markdown is excluded from both its push and pull_request path filters. So the whole-tree formatting gate never ran on #942, and it doesn't run on main pushes for that file either. This PR is the first packages/** PR after #942, so it is simply the first to run prettier --check . against the unformatted file.

Since code-quality is not a required status check, this PR is already mergeable (mergeStateStatus: UNSTABLE, not BLOCKED), and the insert-columns backtick-quoting fix is complete — all functional checks (unit, integration, cluster, cloud) are green.

I've intentionally kept the CODE_OF_CONDUCT.md reformat out of this bug PR to keep it to a single concern. It's best fixed at source on main (npx prettier --write CODE_OF_CONDUCT.md), which would also unblock the whole-tree gate for every future packages/** PR; I've flagged that as a separate follow-up.