ClickHouse / ClickHouse/clickhouse-js

insert(): table parameter is concatenated raw into SQL — table names needing backtick quoting cannot be used

Open
#1,004 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
331
Forks
74
PR merge metrics
No merged PRs in 30d

Description

## Description

`client.insert()` builds its statement by concatenating the raw `table` value:

```ts
// packages/client-common/src/client.ts:745 (getInsertQuery)
return `INSERT INTO ${params.table.trim()}${columnsPart} FORMAT ${format}`;
```

The JSDoc for the parameter says only *"Name of a table to insert into."* (`packages/client-common/src/client.ts:177`), which reads as a raw identifier. But because the value is spliced into SQL verbatim, any legal ClickHouse table name that requires identifier quoting (`my-table`, `user events`, a name starting with a digit, …) produces a server-side `SYNTAX_ERROR` even though the table exists — `INSERT INTO my-table` parses as `my` minus `table`.

The workaround is to pre-quote (`` table: '`my-table`' ``), which works today precisely because the value is passed through untouched. So the parameter's real contract is "SQL fragment", not "name" — it just isn't documented as one. Either contract is defensible; the ask is to settle it:

1. **Raw name** (what the doc implies): backtick-quote/escape internally, with pass-through for already-quoted input so the existing pre-quoting workaround keeps working (JDBC `Statement.enquoteIdentifier` has the same contract). Note `db.table` is a common value for this parameter, so naive whole-string quoting would be a breaking change — a fix would need to split on the qualifier boundary.
2. **SQL fragment**: keep the behavior and document that the caller must quote names that need it (and that the value is interpolated into SQL, i.e. it must never come from untrusted input).

### Relationship to existing work

- #945 / #949 cover the sibling problem for the **`columns`** parameter. PR #949 backtick-quotes the column identifiers but deliberately leaves `params.table.trim()` untouched, so the `table` half described here survives that fix. Filing separately rather than commenting on #945, since the resolution for `table` is not the same (the `db.table` qualifier case makes blind quoting unsafe).
- `getTableSchema` from the upstream Java report has no counterpart in this client, so only the insert path applies here.

## ClickHouse server version

`26.8.1.2041` (local single node, HTTP). Verified against a running server.

## Reproduction

Integration test (vitest, `packages/client-node/__tests__/integration/`), run with `npm run test:integration -- run `:

```ts
import type { ClickHouseClient } from "@clickhouse/client-common";
import { describe, it, beforeEach, afterEach, expect } from "vitest";
import { createTestClient } from "@test/utils/client";
import { guid } from "@test/utils/guid";

describe("[Node.js] insert into a table whose name needs quoting", () => {
let client: ClickHouseClient;
let tableName: string;

beforeEach(async () => {
client = createTestClient();
tableName = `scratch-quoted-${guid()}`; // legal name, needs backquotes
await client.command({
query: `CREATE TABLE \`${tableName}\` (id UInt32) ENGINE MergeTree ORDER BY id`,
});
});

afterEach(async () => {
await client.close();
});

it("accepts the raw table name", async () => {
await client.insert({
table: tableName,
values: [{ id: 42 }],
format: "JSONEachRow",
});
const rs = await client.query({
query: `SELECT * FROM \`${tableName}\``,
format: "JSONEachRow",
});
expect(await rs.json()).toEqual([{ id: 42 }]);
});

it("accepts a pre-quoted table name (workaround)", async () => {
await client.insert({
table: `\`${tableName}\``,
values: [{ id: 43 }],
format: "JSONEachRow",
});
const rs = await client.query({
query: `SELECT * FROM \`${tableName}\``,
format: "JSONEachRow",
});
expect(await rs.json()).toEqual([{ id: 43 }]);
});
});
```

**Expected:** both tests pass (the table exists, and the parameter is documented as a name).

**Actual:** the pre-quoted test passes; the raw-name test fails —

```
Tests 1 failed | 1 passed (2)

FAIL ... > accepts the raw table name
Error: Syntax error: failed at position 20 (-) (line 1, col 20): -quoted-26a1fd0e... FORMAT JSONEachRow
{"id":42}
. Expected one of: token, Dot, OpeningRoundBracket, FROM INFILE, SETTINGS, VALUES, FORMAT, SELECT, WITH, FROM.
code: '62', type: 'SYNTAX_ERROR'
```

## Suggested fix

`getInsertQuery` in `packages/client-common/src/client.ts:730-746` — apply the same identifier-quoting helper introduced by #949 to `params.table`, splitting on the `database.table` boundary and passing through segments that are already backtick- or double-quoted; alternatively, document `table` as a SQL fragment in the `InsertParams.table` JSDoc (`packages/client-common/src/client.ts:176-177`) and in the insert docs.

## Link

Relayed from https://github.com/ClickHouse/clickhouse-java/issues/3089

Contributor guide

Open the contributing guide

Research direction

Read `getInsertQuery` and the `InsertParams.table` JSDoc in `packages/client-common/src/client.ts`, then compare the existing identifier-quoting work in #949. Decide whether `table` is a raw name or SQL fragment, considering qualified and already-quoted names; validate the chosen contract with an integration test under `packages/client-node/__tests__/integration/`, run using `npm run test:integration -- run `.

Written by the indexing model from the issue text.

Assessment

Tech stack
clickhouse, typescript
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.