Skip to content

Fix None rendering as \N inside Array/Tuple/Map bind parameters#883

Open
polyglotAI-bot wants to merge 1 commit into
mainfrom
polyglot/nested-none-bind-null
Open

Fix None rendering as \N inside Array/Tuple/Map bind parameters#883
polyglotAI-bot wants to merge 1 commit into
mainfrom
polyglot/nested-none-bind-null

Conversation

@polyglotAI-bot

Copy link
Copy Markdown
Collaborator

Description

Fixes #879.

format_bind_value (clickhouse_connect/driver/binding.py) formats Python values for server-side {name:Type} bind parameters. Its None branch unconditionally returned the escaped-text sentinel \N. \N is the correct way to pass a NULL for a top-level scalar Nullable parameter, but inside an Array, Tuple, or Map literal the server parses a SQL expression and expects the keyword NULL. So {tup:Tuple(String, Nullable(String))} bound to ("user_1", None) produced ('user_1', \N) and the server rejected it with:

Code: 26. DB::Exception: Cannot parse quoted string: expected opening quote ''', got '\':
value ('user_1', \N) cannot be parsed as Tuple(String, Nullable(String)) ... (CANNOT_PARSE_QUOTED_STRING)

The fix emits NULL when nested (top_level=False, the level recurse() already uses for container elements) and keeps \N at top level. This mirrors how the sibling client-side path format_query_value already renders None as NULL, and matches the recent nested-value fix for UUID/IP in the same function (#792).

Changes

  • clickhouse_connect/driver/binding.py: in format_bind_value, the None branch now returns "\\N" if top_level else "NULL". This resolves Array, Tuple, and nested containers on the default path, and Map values when dict_parameter_format is map. The default json dict path is unaffected (it serializes None as JSON null via any_to_json, never through this branch).

Test

  • tests/integration_tests/test_params.py::test_null_in_containers (runs sync and async via the shared param_client/call fixtures): round-trips None inside Tuple(String, Nullable(String), Int32), Array(Nullable(String)), and Array(Tuple(String, Nullable(String))) against a live server and asserts the returned values. Fails on main with CANNOT_PARSE_QUOTED_STRING; passes with the fix.
  • tests/unit_tests/test_driver/test_params.py: new parametrized test_format_bind_value cases for nested None (array, tuple with the null mid-row and a trailing field, array-of-tuple) plus an explicit top-level (None, "\\N") contrast case that pins the unchanged scalar behavior. New test_format_bind_value_map_null covers the dict_parameter_format="map" path.

Expected values were confirmed against a local ClickHouse server (26.5.1.882).

Pre-PR validation gate

  • Deterministic repro confirmed (fails on main, passes on branch)
  • Root cause documented above
  • Fix targets the root cause
  • Test fails without fix, passes with fix (verified in both directions)
  • No existing tests broken; existing top-level None behavior preserved
  • CHANGELOG updated
  • Convention compliance verified per AGENTS.md (ruff clean, double quotes, neutral test values)

Notes

Out of scope for this fix: passing a dict to a {name:Map(...)} parameter with the default dict_parameter_format="json" renders JSON (double-quoted) which the server also rejects for a Map literal. That is a separate, pre-existing issue independent of the \N-vs-NULL defect and is not addressed here.

format_bind_value emitted the escaped-text sentinel \N for None at every
nesting level. \N is only valid for a top-level scalar Nullable bind
parameter; inside an Array/Tuple/Map literal the server parses a SQL
expression and expects the NULL keyword, so a parameter such as
{tup:Tuple(String, Nullable(String))} bound to ("user_1", None) was
rejected with Code: 26 CANNOT_PARSE_QUOTED_STRING. Emit NULL when nested,
keep \N at top level.

Fixes: #879

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes server-side typed bind parameters ({name:Type}) so that Python None nested inside container literals is rendered as the SQL NULL keyword (instead of the top-level \N sentinel), resolving ClickHouse parse failures for Array, Tuple, and Map bind values.

Changes:

  • Update format_bind_value to emit "NULL" for nested None values while preserving top-level scalar None -> "\\N" behavior.
  • Add unit tests covering nested None formatting (including the dict_parameter_format="map" path) and a contrast case for top-level None.
  • Add an integration regression test that round-trips None inside Tuple(...), Array(Nullable(...)), and Array(Tuple(...)) using the shared sync/async fixtures, and document the user-visible fix in CHANGELOG.md.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
clickhouse_connect/driver/binding.py Fix nested-None rendering for typed bind parameters by emitting NULL inside container literals while keeping top-level \N.
tests/unit_tests/test_driver/test_params.py Add unit coverage for nested None formatting, including dict_parameter_format="map" behavior and top-level None preservation.
tests/integration_tests/test_params.py Add sync/async integration regression coverage for nested None in Tuple and Array typed bind parameters.
CHANGELOG.md Document the bug fix and its impact, referencing the reported issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[backfill: ClickHouse/clickhouse-connect] Tuple(Nullable(...)) parameter with None fails: backslash-N rendered inside tuple

2 participants