ClickHouse / ClickHouse/clickhouse-js

query(): trailing semicolon followed by a comment breaks FORMAT append (SYNTAX_ERROR 62)

Open
#973 0 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.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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.