Skip to content

DB-API executemany bulk insert path silently drops function calls and other expressions in the VALUES list #934

Description

@claude

Description

Cursor._try_bulk_insert (clickhouse_connect/dbapi/cursor.py) decides whether an executemany INSERT can be sent through the native block writer (client.insert). It parses the target and the optional column list, then checks only that the remainder contains (on main) / starts with (after #933) the keyword VALUES. It never inspects the contents of the VALUES tuple.

The block writer can only carry a values list made of parameter placeholders — one value per named column. When the values list contains anything else, in particular a function call such as hex(%s) or now(), the fast path is still taken and the SQL expression is discarded:

  • If the placeholder count still matches the column count (VALUES (%s, hex(%s))), the raw parameter is written straight into the column and the function is silently ignored — wrong data, no error.
  • If the expression consumes no placeholder (VALUES (%s, hex(unhex('AB')))), the row is shorter than the column list and the driver raises the misleading ProgrammingError: Insert data column count does not match column names, instead of falling back to the row-by-row client.query path that would execute the statement correctly.

The correct behaviour is what the row-by-row path already does: a values list that is not placeholders-only must keep the original statement intact and go through parameter substitution, so the server evaluates the function.

This is the clickhouse-connect analogue of ClickHouse/clickhouse-java#3027, where an INSERT ... VALUES holding a function call was mis-routed to the RowBinary writer because the function call went undetected. In clickhouse-java the mis-detection depended on an ANTLR4 grammar gap; here there is no detection at all, so every function call in the values list is affected, including ones as ordinary as now().

ClickHouse server version

26.7.2.59 (reproduced against a live server over HTTP).

Reproduction

Live-server repro (clickhouse_connect.dbapi):

from clickhouse_connect.dbapi import connect

conn = connect(host='localhost', port=8123)
cur = conn.cursor()
cur.execute("DROP TABLE IF EXISTS t_func_bug")
cur.execute("CREATE TABLE t_func_bug (v1 Int32, v2 String) Engine MergeTree ORDER BY ()")

# Case A: function wrapping a placeholder, placeholder count matches column count
cur.executemany("INSERT INTO t_func_bug (v1, v2) VALUES (%s, hex(%s))", [(1, 'AB'), (2, 'CD')])
cur.execute("SELECT v1, v2 FROM t_func_bug ORDER BY v1")
print("A:", cur.fetchall())        # expected [(1, '4142'), (2, '4344')]

cur.execute("TRUNCATE TABLE t_func_bug")

# Case B: literal function call, no placeholder for that column
try:
    cur.executemany("INSERT INTO t_func_bug (v1, v2) VALUES (%s, hex(unhex('AB')))", [(1,), (2,)])
except Exception as e:
    print("B raised:", type(e).__name__, e)
cur.execute("SELECT v1, v2 FROM t_func_bug ORDER BY v1")
print("B:", cur.fetchall())        # expected [(1, 'AB'), (2, 'AB')]

Actual output on main:

A: [(1, 'AB'), (2, 'CD')]          <-- hex() silently ignored, raw parameter stored
B raised: ProgrammingError Insert data column count does not match column names
B: []                              <-- nothing inserted

Expected output:

A: [(1, '4142'), (2, '4344')]
B: [(1, 'AB'), (2, 'AB')]

The same can be shown without a server, which makes clear the statement is being routed to client.insert rather than client.query:

from unittest.mock import Mock
from clickhouse_connect.dbapi.cursor import Cursor

client = Mock()
cursor = Cursor(client)
cursor.executemany("INSERT INTO tbl (v1, v2) VALUES (%s, hex(%s))", [(1, 'AB')])
print(client.insert.call_args, "queries:", client.query.call_count)
# call('tbl', [(1, 'AB')], ['v1', 'v2'], settings=None) queries: 0
# expected: insert not called, one query per row instead

Suggested fix

In _try_bulk_insert (clickhouse_connect/dbapi/cursor.py:142-178), after establishing that the remainder starts with VALUES, parse the values tuple and require every element to be a bare parameter placeholder (%s / %(name)s), returning False otherwise so the statement stays on the row-by-row path. parse_callable, already imported there, can supply the tuple contents. A trailing second tuple or any other extra text after the single values tuple should likewise disqualify the fast path.

Note this is not covered by #932 / #933: those address parsing of the INSERT target and column list, and #933 still only checks temp.upper().startswith("VALUES") without looking inside the tuple.

Link

Relayed from ClickHouse/clickhouse-java#3027

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