Skip to content

Commit aa8bd24

Browse files
feat(backend/kernel): wire TSparkParameter through to kernel bind_param
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>
1 parent 8958e76 commit aa8bd24

4 files changed

Lines changed: 233 additions & 14 deletions

File tree

src/databricks/sql/backend/kernel/client.py

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,6 @@
1414
1515
Phase 1 gaps documented in the integration design:
1616
17-
- Parameter binding (``parameters=[TSparkParameter, ...]``) is not
18-
yet supported — the PyO3 ``Statement`` doesn't expose
19-
``bind_param``. ``execute_command(parameters=[...])`` raises
20-
``NotSupportedError``.
2117
- ``query_tags`` on execute is not supported (kernel exposes
2218
``statement_conf`` but PyO3 doesn't surface it).
2319
- ``get_tables`` with a non-empty ``table_types`` filter applies
@@ -262,11 +258,6 @@ def execute_command(
262258
) -> Union["ResultSet", None]:
263259
if self._kernel_session is None:
264260
raise InterfaceError("Cannot execute_command without an open session.")
265-
if parameters:
266-
raise NotSupportedError(
267-
"Parameter binding is not yet supported on the kernel backend "
268-
"(PyO3 Statement.bind_param lands in a follow-up PR)."
269-
)
270261
if query_tags:
271262
raise NotSupportedError(
272263
"Statement-level query_tags are not yet supported on the kernel backend."
@@ -275,6 +266,13 @@ def execute_command(
275266
stmt = self._kernel_session.statement()
276267
try:
277268
stmt.set_sql(operation)
269+
if parameters:
270+
# Lazy import — type_mapping touches pyarrow at
271+
# module load; keep `execute_command` callable from
272+
# contexts that don't yet need it.
273+
from databricks.sql.backend.kernel.type_mapping import bind_tspark_params
274+
275+
bind_tspark_params(stmt, parameters)
278276
if async_op:
279277
async_exec = stmt.submit()
280278
command_id = CommandId.from_sea_statement_id(async_exec.statement_id)

src/databricks/sql/backend/kernel/type_mapping.py

Lines changed: 61 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,20 @@
66
flattens the conversion so ``KernelResultSet`` and any future
77
kernel-result wrapper share the same mapping.
88
9-
Parameter binding (``TSparkParameter`` → kernel ``TypedValue``) is
10-
not yet implemented — the PyO3 ``Statement`` doesn't expose a
11-
``bind_param`` method on this branch. It'll land in a follow-up
12-
once that PyO3 surface ships.
9+
Parameter binding (``TSparkParameter`` → kernel
10+
``Statement.bind_param``) is handled by ``bind_tspark_params``
11+
forwards the connector's already-string-encoded form to the kernel
12+
binding without an intermediate Python-typed round-trip.
1313
"""
1414

1515
from __future__ import annotations
1616

17-
from typing import List, Tuple
17+
from typing import Any, List, Tuple
1818

1919
import pyarrow
2020

21+
from databricks.sql.thrift_api.TCLIService import ttypes
22+
2123

2224
def _arrow_type_to_dbapi_string(arrow_type: pyarrow.DataType) -> str:
2325
"""Map a pyarrow type to the Databricks SQL type name used in
@@ -77,3 +79,57 @@ def description_from_arrow_schema(schema: pyarrow.Schema) -> List[Tuple]:
7779
)
7880
for field in schema
7981
]
82+
83+
84+
def _tspark_param_value_str(param: ttypes.TSparkParameter) -> Any:
85+
"""Extract the string-encoded value from a ``TSparkParameter``,
86+
or ``None`` for SQL NULL.
87+
88+
Native parameters (``IntegerParameter`` etc.) always wrap their
89+
value in ``TSparkParameterValue(stringValue=str(self.value))``;
90+
``VoidParameter`` sets ``stringValue="None"`` but the type is
91+
``"VOID"`` — the kernel-side parser ignores the value when the
92+
type is VOID, so we don't have to special-case here.
93+
"""
94+
if param.value is None:
95+
return None
96+
return param.value.stringValue
97+
98+
99+
def bind_tspark_params(
100+
kernel_stmt, parameters: List[ttypes.TSparkParameter]
101+
) -> None:
102+
"""Bind a list of ``TSparkParameter`` onto a kernel ``Statement``.
103+
104+
The kernel expects positional bindings only (SEA v0 doesn't
105+
accept named bindings on the wire). The connector's
106+
``TSparkParameter`` has an ``ordinal: bool`` flag; ``True`` means
107+
"treat as positional in source-list order". Native bindings
108+
almost always come through positional today; named-binding
109+
parameters surface as ``NotSupportedError`` so the user gets a
110+
clear message instead of a server-side rejection.
111+
112+
Compound types (``ARRAY`` / ``MAP`` / ``STRUCT``) are routed
113+
through the kernel parser which currently rejects them — same
114+
user-visible message ("compound parameter types … are not yet
115+
supported"). Tracked as a follow-up.
116+
"""
117+
for i, param in enumerate(parameters, start=1):
118+
# The connector's `ordinal` field is a bool (True/False) on
119+
# native params and indicates positional vs named. Named
120+
# params can't flow through the kernel today; raise early
121+
# rather than letting the server reject.
122+
if getattr(param, "ordinal", None) is False and getattr(param, "name", None):
123+
from databricks.sql.exc import NotSupportedError
124+
125+
raise NotSupportedError(
126+
f"Named parameter binding (got name={param.name!r}) is not yet "
127+
"supported on the kernel backend; pass parameters positionally."
128+
)
129+
130+
sql_type = param.type or "STRING"
131+
value_str = _tspark_param_value_str(param)
132+
# The kernel takes 1-based ordinals; `i` is already that.
133+
# Errors from the kernel side (bad literal, unsupported type,
134+
# etc.) come up as KernelError and bubble through normally.
135+
kernel_stmt.bind_param(i, value_str, sql_type)

tests/e2e/test_kernel_backend.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,3 +184,69 @@ def test_bad_sql_surfaces_as_databaseerror(conn):
184184
# Structured fields copied off the kernel exception:
185185
assert getattr(err, "code", None) == "SqlError"
186186
assert getattr(err, "sql_state", None) == "42P01"
187+
188+
189+
# ── Parameter binding ─────────────────────────────────────────────
190+
191+
192+
def test_parameterized_query_round_trips(conn):
193+
"""Positional parameter binding via the kernel backend. The
194+
connector's native parameter classes (IntegerParameter etc.)
195+
serialize to TSparkParameter under the hood; the kernel
196+
backend's mapper forwards them positionally to the kernel.
197+
"""
198+
from databricks.sql.parameters.native import (
199+
IntegerParameter,
200+
StringParameter,
201+
BooleanParameter,
202+
)
203+
204+
with conn.cursor() as cur:
205+
cur.execute(
206+
"SELECT ? AS i, ? AS s, ? AS b",
207+
[
208+
IntegerParameter(42),
209+
StringParameter("alice"),
210+
BooleanParameter(True),
211+
],
212+
)
213+
rows = cur.fetchall()
214+
assert len(rows) == 1
215+
assert rows[0][0] == 42
216+
assert rows[0][1] == "alice"
217+
assert rows[0][2] is True
218+
219+
220+
def test_parameterized_query_with_null(conn):
221+
"""`None` in the parameter list flows through as VoidParameter
222+
→ kernel TypedValue::Null."""
223+
with conn.cursor() as cur:
224+
cur.execute("SELECT ? IS NULL AS is_null", [None])
225+
rows = cur.fetchall()
226+
assert rows[0][0] is True
227+
228+
229+
def test_parameterized_query_decimal(conn):
230+
"""DECIMAL parameters carry precision/scale in the SQL type
231+
string ('DECIMAL(p,s)') — the kernel parser extracts them so
232+
fractional digits survive the wire.
233+
234+
Uses the connector's auto-inference path
235+
(`calculate_decimal_cast_string`) to derive precision/scale
236+
from the value; the explicit-arg path
237+
(`DecimalParameter(v, scale=, precision=)`) has a pre-existing
238+
bug in this branch where the format-args are passed
239+
`(scale, precision)` instead of `(precision, scale)` — out of
240+
scope for this PR.
241+
"""
242+
import decimal
243+
from databricks.sql.parameters.native import DecimalParameter
244+
245+
with conn.cursor() as cur:
246+
cur.execute(
247+
"SELECT ? AS d",
248+
[DecimalParameter(decimal.Decimal("-123.45"))],
249+
)
250+
rows = cur.fetchall()
251+
# Server echoes back as decimal.Decimal.
252+
assert str(rows[0][0]) == "-123.45"

tests/unit/test_kernel_type_mapping.py

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,102 @@ def test_description_from_schema_preserves_field_names_and_order():
7171
for d in desc:
7272
assert len(d) == 7
7373
assert d[2:] == (None, None, None, None, None)
74+
75+
76+
# ─── bind_tspark_params ──────────────────────────────────────────────────
77+
78+
79+
def _mk_param(*, type, value, ordinal=True, name=None):
80+
"""Build a minimal TSparkParameter for tests."""
81+
from databricks.sql.thrift_api.TCLIService import ttypes
82+
83+
p = ttypes.TSparkParameter(ordinal=ordinal, name=name, type=type)
84+
p.value = ttypes.TSparkParameterValue(stringValue=value) if value is not None else None
85+
return p
86+
87+
88+
class _RecordingStmt:
89+
"""Stand-in for the kernel `Statement` pyclass — records every
90+
`bind_param` call so tests can assert the (ordinal, value, type)
91+
triples the mapper forwarded."""
92+
93+
def __init__(self):
94+
self.calls = []
95+
96+
def bind_param(self, ordinal, value_str, sql_type):
97+
self.calls.append((ordinal, value_str, sql_type))
98+
99+
100+
def test_bind_tspark_params_forwards_each_param_positionally():
101+
from databricks.sql.backend.kernel.type_mapping import bind_tspark_params
102+
103+
params = [
104+
_mk_param(type="INT", value="42"),
105+
_mk_param(type="STRING", value="alice"),
106+
_mk_param(type="DATE", value="2026-05-15"),
107+
]
108+
stmt = _RecordingStmt()
109+
bind_tspark_params(stmt, params)
110+
assert stmt.calls == [
111+
(1, "42", "INT"),
112+
(2, "alice", "STRING"),
113+
(3, "2026-05-15", "DATE"),
114+
]
115+
116+
117+
def test_bind_tspark_params_null_value():
118+
"""TSparkParameter with value=None → kernel sees value_str=None,
119+
interpreted as SQL NULL regardless of the SQL type."""
120+
from databricks.sql.backend.kernel.type_mapping import bind_tspark_params
121+
122+
p = _mk_param(type="STRING", value=None)
123+
stmt = _RecordingStmt()
124+
bind_tspark_params(stmt, [p])
125+
assert stmt.calls == [(1, None, "STRING")]
126+
127+
128+
def test_bind_tspark_params_void_passes_through():
129+
"""VoidParameter sets type='VOID' with stringValue='None'; the
130+
kernel parser ignores the value when type=VOID."""
131+
from databricks.sql.backend.kernel.type_mapping import bind_tspark_params
132+
133+
p = _mk_param(type="VOID", value="None")
134+
stmt = _RecordingStmt()
135+
bind_tspark_params(stmt, [p])
136+
assert stmt.calls == [(1, "None", "VOID")]
137+
138+
139+
def test_bind_tspark_params_named_param_rejected():
140+
"""The kernel doesn't accept named bindings on the SEA wire;
141+
surface that at the connector layer so the user gets a pointed
142+
error instead of a server-side rejection."""
143+
from databricks.sql.backend.kernel.type_mapping import bind_tspark_params
144+
from databricks.sql.exc import NotSupportedError
145+
146+
p = _mk_param(type="INT", value="42", ordinal=False, name="my_param")
147+
stmt = _RecordingStmt()
148+
with pytest.raises(NotSupportedError, match="(?i)named"):
149+
bind_tspark_params(stmt, [p])
150+
# Nothing should have been forwarded before the rejection.
151+
assert stmt.calls == []
152+
153+
154+
def test_bind_tspark_params_missing_type_defaults_to_string():
155+
"""Defensive: a TSparkParameter with no `type` shouldn't crash
156+
the mapper — fall back to STRING and let the kernel parse."""
157+
from databricks.sql.backend.kernel.type_mapping import bind_tspark_params
158+
from databricks.sql.thrift_api.TCLIService import ttypes
159+
160+
p = ttypes.TSparkParameter(ordinal=True, name=None, type=None)
161+
p.value = ttypes.TSparkParameterValue(stringValue="hello")
162+
stmt = _RecordingStmt()
163+
bind_tspark_params(stmt, [p])
164+
assert stmt.calls == [(1, "hello", "STRING")]
165+
166+
167+
def test_bind_tspark_params_empty_list_is_noop():
168+
from databricks.sql.backend.kernel.type_mapping import bind_tspark_params
169+
170+
stmt = _RecordingStmt()
171+
bind_tspark_params(stmt, [])
172+
assert stmt.calls == []

0 commit comments

Comments
 (0)