ClickHouse / ClickHouse/clickhouse-go
bind: $ is not a name character in @name placeholders, so $-containing parameter names are unusable and the wrong parameter can be silently substituted
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 3.3k
- Forks
- 684
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 14
Description
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:
- A parameter whose name contains
$cannot be bound at all.bind("SELECT @id$x AS v", Named("id$x", 42))fails withhave no arg for "@id" param— the nameid$xis in the params map, but the scanner only ever looks up@id. Same for a trailing$(@id$) and in aWHEREposition (@a$b). - The wrong parameter is silently substituted. With both
idandid$xdefined,SELECT @id$x AS vbecomesSELECT 7$x AS v— the value ofid, notid$x, with no error. This is a silent wrong-value bug, not a syntax error. - A reference to an undefined parameter is silently rewritten instead of reported. With only
iddefined,SELECT @id$xbecomesSELECT 42$xand no error is returned, whereas the analogousSELECT @id2andSELECT @id_xcorrectly fail withhave 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$. - A leading
$(@$x) is not recognized as a placeholder at all: the text is returned verbatim with no error, so the@$xreaches 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:
isNameCharis only used bybindNamed, so numeric ($N) and positional (?) binding are unaffected;bindParamsFormats/bindNumericdetect$+ digit separately and should keep doing so (@is required before a named placeholder, so there is no ambiguity with$1).- The doc comment on
isNameCharreferencing 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).
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with isNameChar and bindNamed in bind.go:230-235 and 411-430, then run the reproduction in bind_dollar_test.go with go test -run TestDollarInNamedParam -v .. Treat dollar signs as part of named placeholders and update the related comment; done means all listed dollar-containing cases bind or report the full parameter name while the existing @id2 and @id_x behavior stays unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100