Target client repo

ClickHouse/clickhouse-go

Severity

sev:1 — Easy client-side workaround (use clickhouse.WithSettings(...) via context, the idiomatic API). The bug is silent (no error) but only triggers on the non-idiomatic path of embedding SETTINGS directly in the PrepareBatch SQL string.

Description

The source bug (clickhouse-java #1152) reports that the position of SETTINGS in an INSERT matters for the JDBC client. The same class of bug exists in clickhouse-go, in a slightly different shape:

PrepareBatch(ctx, query) runs query through extractNormalizedInsertQueryAndColumns in batch.go:14, which uses the regex (batch.go:9):

(?i)(?:(?:--[^\n]*|#![^\n]*|#\s[^\n]*)\n\s*)*(INSERT\s+INTO\s+([^(]+)(?:\s*\([^()]*(?:\([^()]*\)[^()]*)*\))?)(?:\s*VALUES)?

After truncating trailing VALUES … and FORMAT …, the regex captures group 1 = INSERT INTO <name>[(cols)] and group 2 = <name>. The normalized query sent to ClickHouse is <group1> FORMAT Native. Anything appearing after the column list (e.g. a SETTINGS … clause) is silently dropped.

Observed outputs from extractNormalizedInsertQueryAndColumns:

Net effect: a user who writes PrepareBatch(ctx, \"INSERT INTO foo (a, b) SETTINGS async_insert=1\") (mirroring the documented SQL placement of SETTINGS between the column list and VALUES) will see async_insert (or any other inline setting) silently ignored on the server, identical in spirit to the JDBC report — the position of SETTINGS matters. The canonical Go API for this is clickhouse.Context(ctx, clickhouse.WithSettings(clickhouse.Settings{...})), which works correctly; the inline-SQL path is what's broken.

ClickHouse server version

26.4.2 (testcontainers image started by the project's own test harness during the reproduction below).

Reproduction

Drop the following into a new file target-repo/issue_settings_test.go in the root clickhouse package and run go test -run TestSettingsPositionInPrepareBatch -v .:

package clickhouse

import "testing"

func TestSettingsPositionInPrepareBatch(t *testing.T) {
    cases := []struct {
        name, query string
    }{
        {"settings before column list",         "INSERT INTO foo SETTINGS async_insert=1"},
        {"settings after column list",          "INSERT INTO foo (a, b) SETTINGS async_insert=1"},
        {"settings between column list+VALUES", "INSERT INTO foo (a, b) SETTINGS async_insert=1 VALUES"},
    }
    for _, c := range cases {
        nq, tn, cols, err := extractNormalizedInsertQueryAndColumns(c.query)
        if err != nil { t.Fatalf("%s: %v", c.name, err) }
        t.Logf("%s: normalized=%q table=%q cols=%v", c.name, nq, tn, cols)
    }
}

Output:

settings before column list:         normalized="INSERT INTO foo SETTINGS async_insert=1 FORMAT Native" table="foo SETTINGS async_insert=1" cols=[]
settings after column list:          normalized="INSERT INTO foo (a, b) FORMAT Native"                  table="foo"                         cols=[a b]
settings between column list+VALUES: normalized="INSERT INTO foo (a, b) FORMAT Native"                  table="foo"                         cols=[a b]

Expected: in cases 2 and 3, the SETTINGS async_insert=1 clause should survive into the normalized query (it is valid ClickHouse SQL between the column list and VALUES). Actual: it is silently stripped.

Dedup search

gh issue list --repo ClickHouse/clickhouse-go --search "SETTINGS position" --state all --limit 20   # 13 results, none matching
gh issue list --repo ClickHouse/clickhouse-go --search "PrepareBatch SETTINGS" --state all --limit 20   # 12 results, none matching this regex/silent-drop issue
gh pr list    --repo ClickHouse/clickhouse-go --search "SETTINGS position" --state all --limit 20   # 1 unrelated
gh issue list --repo ClickHouse/integrations-ai-playground --label backfill --label "target:clickhouse-go" --search "SETTINGS"      --state all --limit 20   # 3 unrelated (compression/long-params/JSON)
gh issue list --repo ClickHouse/integrations-ai-playground --label backfill --label "target:clickhouse-go" --search "async_insert"  --state all --limit 20   # 0 results

Suggested fix

The regex in batch.go:9 (and the duplicate in conn_batch.go:19) should be extended to capture an optional SETTINGS … clause between the column list and the (optional) VALUES keyword, so the normalized query preserves it. Equivalently, the function could leave the original query intact and only append FORMAT Native (after stripping any trailing FORMAT … / VALUES …) when SETTINGS or other clauses are present. A test fixture covering all three SETTINGS positions belongs in batch_test.go alongside the existing column-list cases (see batch_test.go:399).

The same parser is reused on the HTTP path (conn_http_batch.go:77), so a fix in batch.go covers both transports.

Source bug

ClickHouse/clickhouse-java#1152