feat(sea): Thrift-parity — params (positional + named), intervals, getInfo, SQL-error class#403
Open
msrathore-db wants to merge 3 commits into
Open
Conversation
Three driver-side parity fixes validated via the node comparator (thrift-vs-SEA on a live warehouse); params/queryTimeout are intentionally out of scope (positional params land via the kernel TypedValue codec). - intervals: default `intervalsAsString: true` on the SEA connection (SeaAuth) so INTERVAL columns surface as strings like the Thrift driver, and map a physical-Utf8 column carrying INTERVAL type-name metadata to STRING in SeaArrowIpc (the kernel keeps the INTERVAL metadata even when rendering as string). Result: interval columns match Thrift in both type code (7) and value. Requires the kernel napi `intervalsAsString` knob. - getInfo: synthesize the three TGetInfoTypes the Thrift server answers — CLI_SERVER_NAME / CLI_DBMS_NAME = "Spark SQL", CLI_DBMS_VER = "3.1.1" — client-side (JDBC DatabaseMetaData style; SEA/kernel has no getInfo RPC), and reject the rest like the server does. New SeaServerInfo.ts. - error class: map kernel `SqlError` (server execution failures — PERMISSION_DENIED, SCHEMA_ALREADY_EXISTS, bad SQL) to OperationStateError(ERROR) instead of the base HiveDriverError, matching the Thrift backend's operation-status error class. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
Removes the M0 param deferral: SeaSessionBackend.executeStatement now forwards positional `?` bindings to the kernel via the napi `ExecuteOptions.positionalParams` surface (kernel #84 / TypedValueInput `{ sqlType, value? }`). New SeaPositionalParams.ts reduces each `DBSQLParameter | value` to that shape — reusing DBSQLParameter's type-inference + stringification — adapting DECIMAL to the parenthesised `DECIMAL(p,s)` form the kernel codec requires and mapping NULL to a value-less VOID input. Also wires `queryTimeout` → `queryTimeoutSecs`. Named params stay rejected (positional-only on the napi surface today). Validated live: all PREPARED_STATEMENT_TYPES cases bind correctly (INT/BIGINT/DECIMAL/STRING/BOOLEAN/DATE/TIMESTAMP/NULL + interval CAST). 201 sea unit tests pass. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
msrathore-db
changed the title
Completes SEA param parity: SeaSessionBackend now forwards `namedParameters`
to the kernel via `ExecuteOptions.namedParams` (kernel `param_named` /
`NamedTypedValueInput {name, sqlType, value?}`), alongside positional. New
`buildSeaNamedParams` reuses the same `toTypedValueInput` mapping (DECIMAL →
DECIMAL(p,s), NULL → VOID). Positional and named are mutually exclusive
(ParameterError, matching the Thrift backend).
Requires kernel napi `namedParams` (extends the positional codec, #84 line).
Validated live: named INT/STRING/DECIMAL/multi-param bind; both-forms rejected.
205 sea unit tests pass.
Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
msrathore-db
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Driver-side SEA↔Thrift parity, validated by the node comparator (thrift-vs-SEA) on a live warehouse. Stacked on
msrathore/sea-execution-metadata(#404, the metadata milestone). The interval + param behaviour needs the kernel napi knobs from databricks-sql-kernel#96 (intervalsAsString) and #84 (positionalParams); the integration branchmsrathore/krn-all-integrationhas both.1. Positional query parameters (wired through napi)
Removes the M0 param deferral.
SeaSessionBackend.executeStatementforwards positional?bindings to the kernel viaExecuteOptions.positionalParams(#84'sTypedValueInput {sqlType, value?}). NewSeaPositionalParams.tsreduces eachDBSQLParameter | valueto that shape — reusingDBSQLParameter's type-inference + stringification — adapting DECIMAL →DECIMAL(p,s)(the codec requires the parenthesised form) and NULL → value-less VOID. Also wiresqueryTimeout → queryTimeoutSecs. Named params stay rejected (napi surface is positional-only). AllPREPARED_STATEMENT_TYPEScases bind correctly live (INT/BIGINT/DECIMAL/STRING/BOOLEAN/DATE/TIMESTAMP/NULL + intervalCAST).2. Intervals → match Thrift
Default
intervalsAsString: trueon the SEA connection (SeaAuth) so INTERVAL columns surface as strings like the Thrift driver, and map a physical-Utf8column carrying INTERVAL type-name metadata →STRINGinSeaArrowIpc. Result: all 32SELECT *columns match Thrift in type and value.3. getInfo → synthesized (JDBC-style)
SeaServerInfo.tssynthesizes the threeTGetInfoTypes the Thrift server answers —CLI_SERVER_NAME/CLI_DBMS_NAME="Spark SQL",CLI_DBMS_VER="3.1.1"— byte-identical to Thrift, and rejects the rest as the server does.4. SQL-error class → match Thrift
Kernel
SqlError(PERMISSION_DENIED, SCHEMA_ALREADY_EXISTS, bad SQL) →OperationStateError(ERROR)instead of baseHiveDriverError, matching the Thrift operation-status error class.Tests
201 sea unit tests pass (new
positionalParams.test.ts,serverInfo.test.ts, interval-mapping inSeaIntervalParity,SqlError→OperationStateErrorinerror-mapping, getInfo +intervalsAsString+ param-forwarding inexecution/auth-*).Stack
sea-integration-foundation→ #404 (metadata) → this.This pull request and its description were written by Isaac.