ClickHouse / ClickHouse/clickhouse-js
query(): trailing semicolon followed by a comment breaks FORMAT append (SYNTAX_ERROR 62)
- Dominant language
- TypeScript
- Stars
- 331
- Forks
- 74
- PR merge metrics
- No merged PRs in 30d
Description
## Description
`client.query()` appends the `FORMAT` clause onto the user's SQL in `formatQuery`, after first calling `removeTrailingSemi`:
https://github.com/ClickHouse/clickhouse-js/blob/main/packages/client-common/src/client.ts#L700-L719
```ts
function formatQuery(query: string, format: DataFormat): string {
query = query.trim();
query = removeTrailingSemi(query);
return query + " \nFORMAT " + format;
}
function removeTrailingSemi(query: string) {
let lastNonSemiIdx = query.length;
for (let i = lastNonSemiIdx; i > 0; i--) {
if (query[i - 1] !== ";") {
lastNonSemiIdx = i;
break;
}
}
...
}
```
`removeTrailingSemi` only strips semicolons that are the **last** characters of the string. When the statement ends with a semicolon *followed by a comment* (`SELECT 1; -- note`), the loop stops at the comment text, the `;` survives in the middle of the query, and the appended `FORMAT` clause becomes a second statement. The server rejects it:
```
Code: 62. DB::Exception: Syntax error (Multi-statements are not allowed)
```
A trailing comment **without** a semicolon works today, because the appended text begins with `\n`, which terminates the line comment before `FORMAT`. So the failing shape is specifically `;` + comment.
This is the JS analogue of case 2 of ClickHouse/clickhouse-connect#907 (there the same `rstrip(";")`-stops-at-the-comment problem stranded a `;` inside a `SELECT * FROM (...) LIMIT 0` metadata wrapper). clickhouse-js has no DB-API cursor layer, so the metadata-re-query symptom does not apply — but the underlying trailing-`;`-plus-comment handling gap does, in the `FORMAT`-append path.
`command()` and `exec()` also call `removeTrailingSemi`, but they append nothing, so a stranded `;` before a trailing comment is harmless there (verified: `command()` with `CREATE DATABASE ...; -- comment` succeeds).
## ClickHouse server version
`26.7.1.1315` (local, HTTP 8123).
## Reproduction
`packages/client-node/__tests__/integration/trailing_comment.test.ts`:
```ts
import { createClient } from "@clickhouse/client";
import { describe, expect, it } from "vitest";
describe("trailing comment / semicolon", () => {
const client = createClient({ url: "http://localhost:8123" });
// PASSES today
it("trailing line comment, no semicolon", async () => {
const rs = await client.query({
query: "SELECT 13 AS a WHERE 0 -- trailing comment",
format: "JSON",
});
expect(await rs.json()).toBeDefined();
});
// FAILS today: ClickHouseError code 62, SYNTAX_ERROR
it("trailing semicolon then comment", async () => {
const rs = await client.query({
query: "SELECT 13 AS a WHERE 0; -- trailing comment",
format: "JSON",
});
expect(await rs.json()).toBeDefined();
});
// FAILS today: ClickHouseError code 62, SYNTAX_ERROR
it("trailing semicolon, newline, comment", async () => {
const rs = await client.query({
query: "SELECT 13 AS a WHERE 0;\n-- trailing comment",
format: "JSON",
});
expect(await rs.json()).toBeDefined();
});
});
```
Run with `npm run test:node:integration -- trailing_comment`.
**Expected:** all three resolve with a `JSON` result set (metadata for column `a`, zero rows).
**Actual:** `2 failed | 1 passed`. Both semicolon-then-comment cases throw:
```
ClickHouseError: Syntax error (Multi-statements are not allowed): failed at position 23 (end of query) (line 1, col 23): ; -- trailing comment
FORMAT JSON. .
code: '62', type: 'SYNTAX_ERROR'
```
The error message shows the malformed query the client built: the `;` is still there, with `FORMAT JSON` appended after the comment.
## Suggested fix
In `packages/client-common/src/client.ts`, `removeTrailingSemi` (line ~706) should skip trailing comments before looking for the semicolon — i.e. strip any run of trailing whitespace, line comments (`-- ...`, `# ...`), and block comments (`/* ... */`), then strip trailing `;`, repeating until the tail is stable. Comment markers and semicolons inside string literals and backtick/double-quoted identifiers must not be touched, so the scan needs to be literal-aware rather than a plain regex on the tail.
Note that whatever the fix does with the trailing comment matters for the no-semicolon case too: dropping the comment is fine, but if it is preserved, `FORMAT` must still land on its own line (as the current `" \nFORMAT "` does).
Possibly worth coordinating with #972, which touches the same `formatQuery` FORMAT-insertion logic for a trailing `SETTINGS` clause.
## Link
Relayed from ClickHouse/clickhouse-connect#907
Contributor guide
Research direction
Start in packages/client-common/src/client.ts, especially formatQuery and removeTrailingSemi, then run packages/client-node/__tests__/integration/trailing_comment.test.ts with npm run test:node:integration -- trailing_comment. Check the existing handling of comments, semicolons, strings, and quoted identifiers before changing the scan. Done means all three JSON-result cases pass without altering comment markers or semicolons inside literals.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- api
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100