Skip to content

bind: $ is not a name character in @name placeholders, so $-containing parameter names are unusable and the wrong parameter can be silently substituted #1955

Description

@claude

Description

$ is a word character in ClickHouse's lexer: it is legal inside identifiers and inside query-parameter names (WITH 1 AS id$x SELECT id$x, SELECT {id$x:Int32} with param_id$x=42, and also leading/trailing $ — all accepted by the server).

The client-side @name binding path does not agree with that. isNameChar in bind.go:230-235 accepts only [a-zA-Z0-9_]:

// isNameChar reports whether ch is valid in a named placeholder (@name); it
// mirrors the previous bindNamedRe pattern `@[a-zA-Z0-9_]+`.
func isNameChar(ch byte) bool {
	return ch == '_' ||
		(ch >= '0' && ch <= '9') ||
		(ch >= 'a' && ch <= 'z') ||
		(ch >= 'A' && ch <= 'Z')
}

bindNamed (bind.go:411-430) scans @ + a maximal run of isNameChar, so @id$x is lexed as the placeholder @id followed by literal text $x — splitting a token the server lexes as the single identifier id$x. Three distinct symptoms follow:

  1. A parameter whose name contains $ cannot be bound at all. bind("SELECT @id$x AS v", Named("id$x", 42)) fails with have no arg for "@id" param — the name id$x is in the params map, but the scanner only ever looks up @id. Same for a trailing $ (@id$) and in a WHERE position (@a$b).
  2. The wrong parameter is silently substituted. With both id and id$x defined, SELECT @id$x AS v becomes SELECT 7$x AS v — the value of id, not id$x, with no error. This is a silent wrong-value bug, not a syntax error.
  3. A reference to an undefined parameter is silently rewritten instead of reported. With only id defined, SELECT @id$x becomes SELECT 42$x and no error is returned, whereas the analogous SELECT @id2 and SELECT @id_x correctly fail with have no arg for ... param. So a typo'd/undefined placeholder is diagnosed when the name ends in a letter, digit or _, but silently mangles the query when it contains $.
  4. A leading $ (@$x) is not recognized as a placeholder at all: the text is returned verbatim with no error, so the @$x reaches the server and fails there.

The native server-side path is unaffected and does agree with the server: bindQueryOrAppendParameters (query_parameters.go:33-38) keys options.parameters by p.Name verbatim, so SELECT {id$x:Int32} with Named("id$x", 42) works fine. The inconsistency is only in the @name client-side rewrite.

ClickHouse server version

Code analysis plus a unit test of bind; not verified against a running server (no ClickHouse instance was reachable in this environment). The server-side behaviour of $ in identifiers/parameter names is quoted from the upstream report, which verified it against 26.5.1.882. All client-side results below are observed output from the test run.

Reproduction

bind_dollar_test.go in the repository root (package clickhouse):

package clickhouse

import (
	"testing"
	"time"
)

func TestDollarInNamedParam(t *testing.T) {
	cases := []struct {
		label string
		query string
		args  []any
	}{
		{"1: param id$x", "SELECT @id$x AS v", []any{Named("id$x", 42)}},
		{"2: only id defined", "SELECT @id$x", []any{Named("id", 42)}},
		{"3: id and id$x", "SELECT @id$x AS v", []any{Named("id", 7), Named("id$x", 42)}},
		{"contrast @id2", "SELECT @id2 AS v", []any{Named("id", 42)}},
		{"contrast @id_x", "SELECT @id_x AS v", []any{Named("id", 42)}},
		{"leading $", "SELECT @$x AS v", []any{Named("$x", 42)}},
		{"trailing $", "SELECT @id$ AS v", []any{Named("id$", 42)}},
		{"where pos", "SELECT 1 WHERE @a$b = 42", []any{Named("a$b", 42)}},
	}
	for _, c := range cases {
		got, err := bind(time.UTC, c.query, c.args...)
		t.Logf("%-22s -> got=%q err=%v", c.label, got, err)
	}
}

go test -run TestDollarInNamedParam -v .actual output:

1: param id$x          -> got=""                     err=have no arg for "@id" param
2: only id defined     -> got="SELECT 42$x"          err=<nil>
3: id and id$x         -> got="SELECT 7$x AS v"      err=<nil>
contrast @id2          -> got=""                     err=have no arg for "@id2" param
contrast @id_x         -> got=""                     err=have no arg for "@id_x" param
leading $              -> got="SELECT @$x AS v"      err=<nil>
trailing $             -> got=""                     err=have no arg for "@id" param
where pos              -> got=""                     err=have no arg for "@a" param

Expected:

1: param id$x          -> got="SELECT 42 AS v"       err=<nil>
2: only id defined     -> got=""                     err=have no arg for "@id$x" param
3: id and id$x         -> got="SELECT 42 AS v"       err=<nil>
contrast @id2          -> got=""                     err=have no arg for "@id2" param   (unchanged)
contrast @id_x         -> got=""                     err=have no arg for "@id_x" param  (unchanged)
leading $              -> got="SELECT 42 AS v"       err=<nil>
trailing $             -> got="SELECT 42 AS v"       err=<nil>
where pos              -> got="SELECT 1 WHERE 42 = 42" err=<nil>

Reached the same way through the public API (query contains no {name:Type}, so it takes the client-side bind path):

rows, err := conn.Query(ctx, "SELECT @id$x AS v", clickhouse.Named("id$x", 42))
// err: have no arg for "@id" param

Suggested fix

Treat $ as a name character so the placeholder ends where the server's lexer ends the word — one line in bind.go:230:

func isNameChar(ch byte) bool {
	return ch == '_' || ch == '$' ||
		(ch >= '0' && ch <= '9') ||
		(ch >= 'a' && ch <= 'z') ||
		(ch >= 'A' && ch <= 'Z')
}

That fixes all four symptoms at once: @id$x becomes one placeholder name, so it binds when defined, and errors as an undefined param when it is not — matching the existing @id2 / @id_x contrast cases, which keep their behaviour. Two things to check when doing it:

  • isNameChar is only used by bindNamed, so numeric ($N) and positional (?) binding are unaffected; bindParamsFormats/bindNumeric detect $ + digit separately and should keep doing so (@ is required before a named placeholder, so there is no ambiguity with $1).
  • The doc comment on isNameChar referencing the old @[a-zA-Z0-9_]+ regex should be updated.

Link

Same class of bug as ClickHouse/clickhouse-cs#516 (found there in the ADO.NET @name{name:Type} rewriter).

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions