Skip to content

Parameters: $ in a parameter name is not recognized by external_bind_re, so server-side binding is skipped (or type hints silently ignored) #936

Description

@claude

Describe the bug

$ is a word character in ClickHouse's lexer: it is legal inside identifiers and inside query-parameter names. Verified against the live server used below (26.7.2.59):

$ curl -sG localhost:8123 --data-urlencode 'query=SELECT {id$x:Int32} AS v' --data-urlencode 'param_id$x=42'
42
$ curl -sG localhost:8123 --data-urlencode 'query=SELECT {$x:Int32} AS v' --data-urlencode 'param_$x=42'
42
$ curl -sG localhost:8123 --data-urlencode 'query=SELECT {id$:Int32} AS v' --data-urlencode 'param_id$=42'
42

clickhouse_connect disagrees, because the placeholder-detection regex at clickhouse_connect/driver/binding.py:19 uses \w, which excludes $:

external_bind_re = re.compile(r"\{(\w+):([^}]+)\}")

bind_query uses that regex for two separate decisions (binding.py:145-176), and $ breaks both:

1. Whether to bind server-side at all. If no placeholder matches, bind_query concludes the query has no server-side placeholders and falls through to the client-side %-formatting path (finalize_query). For a query whose only placeholders contain $, query % params is a no-op (there are no % conversions), so the query is sent verbatim with no param_* values at all and the server rejects it:

Code: 456. DB::Exception: Substitution `id$x` is not set. (UNKNOWN_QUERY_PARAMETER)

Note this is a silent misclassification, not a name-validation error — the user gets an error that says the parameter is unset even though they passed it.

2. Which type hint applies to each parameter. param_types is keyed by the regex-captured names, so in a mixed query (at least one $-free placeholder, which makes matches non-empty and re-enables the server-side path) the $-named parameter is sent, but param_types.get(k) misses it. So _promote_datetime64 and _extract_tz_from_type never run for it, and the DateTime64 precision / timezone hints are silently dropped:

Query Bound value
SELECT {t:DateTime64(6)} AS t 2024-01-02 03:04:05.123456 OK
SELECT {t$x:DateTime64(6)} AS t, {a:Int32} AS a 2024-01-02 03:04:05 microseconds lost

This is the same class of silent truncation that #739 / #773 fixed for the _64 suffix, reintroduced via the name character class.

ClickHouse server version

26.7.2.59 (local, HTTP 8123). Both failures below are observed results, not code inspection.

Steps to reproduce

test_dollar_param.py at the repo root:

from datetime import datetime

import clickhouse_connect
from clickhouse_connect.driver.binding import bind_query


def test_dollar_in_param_name_binds_server_side():
    # `$` is a legal word char in ClickHouse query-parameter names.
    query, params = bind_query("SELECT {id$x:Int32} AS v", {"id$x": 42})
    assert params == {"param_id$x": "42"}  # actual: {}

    client = clickhouse_connect.get_client(host="localhost", port=8123)
    assert client.query("SELECT {id$x:Int32} AS v", parameters={"id$x": 42}).result_rows == [(42,)]


def test_dollar_in_param_name_keeps_datetime64_precision():
    dt = datetime(2024, 1, 2, 3, 4, 5, 123456)
    client = clickhouse_connect.get_client(host="localhost", port=8123)
    rows = client.query(
        "SELECT {t$x:DateTime64(6)} AS t, {a:Int32} AS a",
        parameters={"t$x": dt, "a": 1},
    ).result_rows
    assert rows == [(dt, 1)]  # actual: sub-second precision truncated
$ python -m pytest test_dollar_param.py -q
FAILED test_dollar_param.py::test_dollar_in_param_name_binds_server_side
  AssertionError: assert {} == {'param_id$x': '42'}
  Right contains 1 more item: {'param_id$x': '42'}

FAILED test_dollar_param.py::test_dollar_in_param_name_keeps_datetime64_precision
  At index 0 diff: (datetime.datetime(2024, 1, 2, 3, 4, 5), 1)
                != (datetime.datetime(2024, 1, 2, 3, 4, 5, 123456), 1)
2 failed

Raw bind_query output for the leading/trailing-$ variants, all of which the server accepts natively:

bind_query('SELECT {idx:Int32} AS v',  {'idx': 42})   -> ('SELECT {idx:Int32} AS v',  {'param_idx': '42'})   # ok
bind_query('SELECT {id$x:Int32} AS v', {'id$x': 42})  -> ('SELECT {id$x:Int32} AS v', {})                    # unbound
bind_query('SELECT {$x:Int32} AS v',   {'$x': 42})    -> ('SELECT {$x:Int32} AS v',   {})                    # unbound
bind_query('SELECT {id$:Int32} AS v',  {'id$': 42})   -> ('SELECT {id$:Int32} AS v',  {})                    # unbound

Each of the three unbound cases then fails with Code: 456 ... Substitution ... is not set.

Expected behaviour

A parameter name should be delimited the way the ClickHouse lexer delimits a word, i.e. $ counts as a word character. {id$x:Int32} with parameters={"id$x": 42} should bind param_id$x=42 and return 42, matching what the server already accepts over raw HTTP — and {t$x:DateTime64(6)} should get the same precision promotion as {t:DateTime64(6)}.

Suggested fix

One-line character-class change at clickhouse_connect/driver/binding.py:19, e.g.:

external_bind_re = re.compile(r"\{([\w$]+):([^}]+)\}")

Two things worth checking alongside it:

  • Binary-bind sentinel overlap. bind_query treats a key matching k.startswith("$") and k.endswith("$") and len(k) > 1 as a binary bind (binding.py:139), so names like $x$ are claimed by that convention and can't route to server-side binding. Widening the regex doesn't create the ambiguity, but it makes it reachable, so the precedence deserves an explicit decision (and probably a test).
  • Unicode axis. Python's \w is Unicode-aware by default, so {idé:Int32} currently matches even though the server's word lexer is ASCII-only. That's the opposite error from $ — too broad rather than too narrow. It's mostly unreachable in practice (param_idé is rejected at the URI layer), but if the regex is being touched anyway, [A-Za-z0-9_$] would align it with the server on both axes.

Provenance

Found while checking whether ClickHouse/clickhouse-cs#516 (same root cause — $ excluded from the parameter-name character class — manifesting there in the ADO.NET @name rewriter) applies to this client. It does, at a different layer: this client has no @name rewriting, but its {name:Type} detection regex has the same too-narrow character class.

Related: ClickHouse/clickhouse-cs#516

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions