Skip to content

feat(backend/kernel): wire TSparkParameter through to kernel bind_param#789

Open
vikrantpuppala wants to merge 1 commit into
feat/kernel-backendfrom
feat/kernel-bind-param
Open

feat(backend/kernel): wire TSparkParameter through to kernel bind_param#789
vikrantpuppala wants to merge 1 commit into
feat/kernel-backendfrom
feat/kernel-bind-param

Conversation

@vikrantpuppala
Copy link
Copy Markdown
Contributor

Stacks on #787 (kernel backend). Pairs with kernel PR databricks/databricks-sql-kernel#18.

Summary

Lifts the `NotSupportedError` the kernel backend currently raises for parametrized queries. The kernel-side PyO3 binding for `Statement.bind_param` lands in databricks-sql-kernel#18; this PR wires the connector's `TSparkParameter` shape through to it.

Before this PR, calling `cursor.execute("SELECT ?", [IntegerParameter(42)])` on `use_sea=True` raised:

```
NotSupportedError: Parameter binding is not yet supported on the kernel backend
```

After: positional parameter binding works end-to-end.

What it does

`bind_tspark_params(kernel_stmt, parameters)` in `type_mapping.py` forwards each `TSparkParameter` to the kernel as `(ordinal, value.stringValue, type)`:

  • `ordinal` is the 1-based position in the parameters list (matches kernel's SEA-wire convention).
  • `value.stringValue` is the connector's already-string-encoded value (or `None` for SQL NULL).
  • `type` is the SQL type name the connector produces (`"INT"`, `"DECIMAL(10,2)"`, etc.).

The connector's `ordinal: bool` flag is checked only to reject named bindings — kernel v0 doesn't accept named bindings on the SEA wire, so we surface that at the connector layer with a pointed `NotSupportedError` rather than letting the server reject.

What's supported

Every type the connector's native parameter classes emit:

  • `BOOLEAN`, `TINYINT` / `SMALLINT` / `INT` / `BIGINT`, `FLOAT` / `DOUBLE`
  • `STRING`, `DATE`, `TIMESTAMP` / `TIMESTAMP_NTZ`, `INTERVAL`
  • `DECIMAL(p,s)` (precision/scale carried in the SQL type string; kernel parses them)
  • `VOID` / Python `None`

What's not supported (known gaps, raises `NotSupportedError`)

  • Named bindings (`cursor.execute(":foo = ?", [param_named("foo", ...)])`) — kernel v0 SEA wire is positional-only.
  • Compound types (`ARRAY` / `MAP` / `STRUCT`) — kernel parser rejects; surfaces as `KernelError(InvalidArgument)`.
  • `BINARY` — SEA wire limitation kernel-side; documented workaround is hex-encode + `unhex(?)` in SQL.

Test plan

  • 6 new unit tests in `tests/unit/test_kernel_type_mapping.py` — positional forwarding, None / VOID, named binding rejection, missing-type defaulting, empty list. 45/45 pass.

  • Pre-existing unit suite still green: 625 pass.

  • 3 new live e2e tests in `tests/e2e/test_kernel_backend.py` against dogfood:

    • Mixed-type round-trip (`IntegerParameter`, `StringParameter`, `BooleanParameter`)
    • `None` parameter
    • `DECIMAL` parameter (precision/scale flows through the SQL type string)

    14/14 pass.

  • `cargo test` / `cargo clippy` / `cargo fmt --check` clean on the kernel side (PR Make GetOperationStatus attempts retryable #18).

This pull request and its description were written by Isaac.

Lifts the NotSupportedError that execute_command currently raises
for parametrized queries. The kernel-side PyO3 binding for
Statement.bind_param landed in databricks-sql-kernel#18; this
commit wires the connector's TSparkParameter shape through to it.

Implementation:

- New `bind_tspark_params(kernel_stmt, parameters)` in
  type_mapping.py forwards each TSparkParameter to the kernel as
  `(ordinal, value.stringValue, type)`. ordinal is the 1-based
  position in the parameters list; the connector's `ordinal: bool`
  flag is checked only to reject named bindings (kernel v0 doesn't
  accept them on the wire).
- execute_command no longer raises on `parameters=[...]`. The
  query_tags branch stays — that's a separate gap.

Tests:

- 6 new unit tests in tests/unit/test_kernel_type_mapping.py for
  the mapper:
  - positional forwarding preserves ordering and (ordinal, value, type)
  - None value forwards as SQL NULL
  - VOID passes through verbatim (kernel parser ignores value for VOID)
  - named bindings raise NotSupportedError with a pointed message
  - missing TSparkParameter.type defaults to STRING (defensive)
  - empty parameters list is a no-op
- 3 new e2e tests in tests/e2e/test_kernel_backend.py against
  dogfood:
  - mixed-type round-trip (INT, STRING, BOOLEAN) via the
    connector's native IntegerParameter/StringParameter/BooleanParameter
  - None parameter (VoidParameter → SQL NULL)
  - DECIMAL parameter with precision/scale carried in the SQL type
    string (auto-inferred — explicit-arg path has a pre-existing
    bug in native.py where format-args are swapped)

Unit pass: 45/45 (was 39). Full unit suite: 625 pass.
Live e2e: 14/14 (was 11).

Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
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.

1 participant