ClickHouse / ClickHouse/clickhouse-js

[backfill: ClickHouse/clickhouse-js] client.query() unconditionally appends FORMAT after user SETTINGS clause, producing invalid SQL on older ClickHouse servers

Open
#970 0 comments 0 reactions 1 assignee Claimed by @polyglotAI-bot View on GitHub
Dominant language
TypeScript
Stars
331
Forks
74
PR merge metrics
No merged PRs in 30d

Description

## Target client repo
`ClickHouse/clickhouse-js`

## Severity
**sev:1** — Easy client-side workaround exists (use `client.exec()` or `client.command()` for queries with a trailing `SETTINGS` clause, since those paths do not auto-append `FORMAT`). Only triggers under opt-in usage (user-supplied `SETTINGS` clause in their own query) and modern ClickHouse server parsers (24.x+) accept the resulting `... SETTINGS ... FORMAT ...` ordering for `DESCRIBE` / `format(...)` queries.

## Description
The `client.query()` method always appends `\nFORMAT ` to the user-supplied SQL string. This happens in `packages/client-common/src/client.ts`:

```ts
async query(...) {
const format = params.format ?? 'JSON'
const query = formatQuery(params.query, format) // <— blind append
...
}

function formatQuery(query: string, format: DataFormat): string {
query = query.trim()
query = removeTrailingSemi(query)
return query + ' \nFORMAT ' + format
}
```

There is no detection of a trailing `SETTINGS ...` clause. If the user passes a query that ends in `SETTINGS k = v, ...`, the client emits:

```
SETTINGS k = v, ...
FORMAT JSON
```

This is exactly the same pattern that broke `clickhouse-connect` in the source bug (issue #211). On older ClickHouse server versions (e.g., 23.3 as in the source report), this fails to parse for `DESCRIBE format(...)` because the server parser required `SETTINGS` to be the trailing clause for that statement form. The well-formed query the user wanted was `... FORMAT Native SETTINGS ...` (or `FORMAT` injected before `SETTINGS`).

## ClickHouse server version tested
26.4.2.10 — modern server **accepts** `DESCRIBE format(...) SETTINGS ... FORMAT JSON` and returns HTTP 200, so end-to-end the bug no longer reproduces against a current server. Verified with:

```bash
curl -s -w "HTTP %{http_code}\n" 'http://localhost:8123/' --data-binary \
"DESC format(JSONEachRow, '{\"id\":1}') SETTINGS schema_inference_hints='age LowCardinality(UInt8)', allow_suspicious_low_cardinality_types=1 FORMAT JSON"
# -> HTTP 200
```

The buggy *client behavior* (blind FORMAT append after user SETTINGS) is still present in clickhouse-js as of `main`; whether it surfaces as a parser error depends on the connected server version. Users on long-tail older ClickHouse versions will still hit the original failure mode.

## Reproduction

```ts
import { createClient } from '@clickhouse/client'

const client = createClient({ url: 'http://localhost:8123' })

const q = `DESC format(JSONEachRow, '{"id" : 1, "age" : 25, "name" : "Josh", "status" : null, "hobbies" : ["football", "cooking"]}') SETTINGS schema_inference_hints = 'age LowCardinality(UInt8), status Nullable(String)', allow_suspicious_low_cardinality_types=1`

const rs = await client.query({ query: q, format: 'JSONEachRow' })
console.log(await rs.json())
```

The wire-level query the client sends is:

```
DESC format(JSONEachRow, '...') SETTINGS schema_inference_hints = '...', allow_suspicious_low_cardinality_types=1
FORMAT JSONEachRow
```

**Expected** (against ClickHouse <24 or any future statement form that requires `SETTINGS` to be trailing): the client should inject `FORMAT` before the trailing `SETTINGS` clause, producing `... FORMAT JSONEachRow SETTINGS ...`.

**Actual** (against ClickHouse <24): server returns a syntax error because `SETTINGS` is not the last clause. Against current ClickHouse (26.x), the query succeeds — the bug is masked by server-side parser improvements, not fixed in the client.

## Workaround
Use `client.exec({ query })` or `client.command({ query })` for queries with a trailing `SETTINGS` clause — these paths call `removeTrailingSemi(params.query.trim())` only and do **not** append `FORMAT`, so the user's query is sent verbatim. The trade-off is that `exec()` returns a raw response stream the caller must parse themselves rather than a typed `ResultSet`.

## Suggested fix
In `packages/client-common/src/client.ts`, `formatQuery` should detect a trailing top-level `SETTINGS k = v, ...` clause (outside string literals and parens) and inject `FORMAT ` *before* that clause rather than after. A conservative regex-based approach (matching `\bSETTINGS\b` at top level and splitting there) would handle the common case; a more robust fix would tokenize. Care needed to skip `SETTINGS` occurrences inside string literals (e.g., `... '... SETTINGS ...' ...`) or table options. The same applies symmetrically if a user already provided their own `FORMAT` clause — currently the client would emit two `FORMAT` clauses.

## Source bug
https://github.com/ClickHouse/clickhouse-connect/issues/211

## Dedup searches
```
gh issue list --repo ClickHouse/clickhouse-js --search "SETTINGS FORMAT" --state all --limit 20 # 20 results, none related
gh issue list --repo ClickHouse/clickhouse-js --search "schema_inference_hints" --state all --limit 10 # 0 results
gh pr list --repo ClickHouse/clickhouse-js --search "SETTINGS FORMAT" --state all --limit 10 # 10 unrelated PRs
gh issue list --repo ClickHouse/integrations-ai-playground --label backfill --label "target:clickhouse-js" --search "SETTINGS FORMAT" --state all --limit 10 # 0 results
gh issue list --repo ClickHouse/integrations-ai-playground --label backfill --label "target:clickhouse-js" --search "FORMAT" --state all --limit 20 # 1 result (Array(Date) — unrelated)
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.