Skip to content

feat(sea): TypedValue codec converter for positional params#399

Open
msrathore-db wants to merge 1 commit into
mainfrom
msrathore/sea-parity-params-positional
Open

feat(sea): TypedValue codec converter for positional params#399
msrathore-db wants to merge 1 commit into
mainfrom
msrathore/sea-parity-params-positional

Conversation

@msrathore-db
Copy link
Copy Markdown
Contributor

Summary

  • Add DBSQLParameter.toNapiTypedValue() and a top-level convertOrdinalParametersToTypedValueInputs() helper that translate the Node driver's user-facing { ordinalParameters: [...] } shape into the flat { sqlType, value: string | null } payload the SEA napi codec consumes.
  • Stringification rules (boolean → `TRUE`/`FALSE`, Date → toISOString(), BigInt/Int64 → .toString(), integer-Number → INTEGER, non-integer Number → DOUBLE) are kept in exact lock-step with the Thrift emitter's toSparkParameter(). A regression-lock test pins the two emitters branch-by-branch, so a future refactor that diverges them breaks the test rather than silently breaking parity.

C5 cluster context

This is PR-B of the C5 (params-basic-binding) chain:

  • PR-A (kernel napi codec): databricks-sql-kernel PR Columns with '.' in name break #84 — JS↔napi TypedValue codec.
  • PR-B (this PR): Node-side converter from DBSQLParameter to napi's TypedValueInput.
  • PR-C (driver-test): SEA-gated parity tests via BackendComparator.forEachBackend against databricks/databricks-driver-test.

Each PR is independent and can be reviewed in parallel — they only depend on each other for the end-to-end live-warehouse run.

Why a converter on main (and not full SEA wiring)

The SEA backend abstraction lives in feature branches (PR #378). main doesn't yet have a SeaBackend impl that would consume this codec, so this PR delivers the pure-TS load-bearing logic — the stringification path that bare JS values traverse on the way to the kernel. When the SEA backend abstraction lands on main, the wiring is a one-liner that calls convertOrdinalParametersToTypedValueInputs(options.ordinalParameters) and passes the result to the napi binding's Connection.executeStatement(sql, { positional_params }).

This shape lets the parity test land independently and lets each PR be reviewed for its own quality without one being blocked behind the others.

Quality gates

  • `npm run lint` ✅ (no errors)
  • `npm test tests/unit/DBSQLParameter.test.ts` — 13 passed (2 pre-existing + 11 new)
  • `npx prettier --check` on changed files ✅
  • DCO sign-off
  • Conventional commit (`feat(sea): ...`)
  • `Co-authored-by: Isaac` trailer

Test plan

  • 11 new unit tests covering the 10-type C5 matrix (INT/BIGINT/FLOAT/DOUBLE/STRING/BOOLEAN/DATE/TIMESTAMP/DECIMAL/BINARY-N/A), explicit-type override, VOID/null short-circuits, and the Thrift-vs-napi alignment regression-lock.
  • End-to-end live-warehouse run — covered by PR-C once PR-A lands and the SEA backend abstraction lands on `main`.

This pull request and its description were written by Isaac.

Add toNapiTypedValue() and convertOrdinalParametersToTypedValueInputs()
helpers that translate the Node driver's user-facing
{ ordinalParameters: [...] } shape into the flat
{ sqlType, value: string | null } payload the SEA napi codec
(databricks-sql-kernel/napi/src/params.rs) consumes.

The stringification rules (boolean -> "TRUE"/"FALSE", Date ->
toISOString(), BigInt/Int64 -> .toString(), integer Number -> "INTEGER",
non-integer Number -> "DOUBLE") are kept in lock-step with the Thrift
emitter's toSparkParameter, so a positional value bound on either
backend hits the server with the same wire-level (type-name, value)
pair. A regression-lock test pins this alignment branch-by-branch.

This is the JS-side half of the C5 (params-basic-binding) cluster.
The kernel-napi half lands in databricks-sql-kernel PR #84. SEA-side
wiring of these helpers into Connection.executeStatement(sql, options)
happens once the SEA backend abstraction (PR #378) lands on main.

Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@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