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

[claude]

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

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:

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