[SEA-NodeJS] Surface operation-status fields (displayMessage/diagnosticInfo/errorDetailsJson, numModifiedRows) on getOperationStatus#422
Closed
msrathore-db wants to merge 1 commit into
Conversation
Replace the SEA backend's neutral synthesized operation status with the
real fields the napi kernel handle exposes on a terminal statement:
numModifiedRows, displayMessage, diagnosticInfo, errorDetailsJson.
- OperationStatus (neutral contract) carries the four rich fields.
- SeaOperationBackend.readRichStatusFields() reads them off the terminal
sync Statement (metadata path + sync runAsync:false path once result()
resolves); null-safe and degrades to all-null on the async path and on
bindings predating the accessors. Wired into status() and the
sync/cancellable completion tick.
- SeaOperationLifecycle.seaFinished merges the rich fields into the
synthesized completion tick (lazy thunk) so finished({callback})
consumers see the same surface as getOperationStatus().
- wireSynthesis.synthesizeThriftStatus maps them into
TGetOperationStatusResp (numModifiedRows re-boxed as node-int64;
null -> undefined for Thrift parity).
numModifiedRows is currently null on SEA: the server delivers DML counts
as result-set data (num_affected_rows) rather than in the status object.
A companion kernel change derives it; this wiring surfaces it then with
no further driver change.
Co-authored-by: Isaac
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
msrathore-db
added a commit
that referenced
this pull request
Jun 6, 2026
…essage, diagnosticInfo, errorDetailsJson) Ports the async rich-status work (was #422) onto the consolidated branch: the napi Statement.status() fields the kernel already exposes are now surfaced through getOperationStatus instead of a flat Succeeded (M1 item). Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
Contributor
Author
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
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.
What
Replaces the SEA backend's neutral, synthesized operation status with the real fields the napi kernel handle exposes on a terminal statement. Before this change,
SeaOperationLifecycle.synthesizeFinishedStatus()returned a fixed{ state: Succeeded, hasResultSet: true }with a comment thatnumModifiedRows/displayMessage/ etc. were "deferred to M1", sogetOperationStatus()on SEA reported nothing beyond the state. The napiStatementalready exposesnumModifiedRows()/displayMessage()/diagnosticInfo()/errorDetailsJson(); the JS side now reads and surfaces them.Changes
lib/contracts/OperationStatus.ts— the backend-neutralOperationStatuscarries the four rich fields (numModifiedRows,displayMessage,diagnosticInfo,errorDetailsJson).lib/sea/SeaOperationBackend.ts— newreadRichStatusFields()reads the accessors off the terminal syncStatement(the metadata path, and the syncrunAsync:falsepath onceresult()has resolved). Null-safe: the async submit path never produces a terminalStatement(theAsyncStatement/AsyncResultHandledon't expose these accessors), and older bindings that predate the accessors both degrade to all-null rather than throwing. Wired intostatus()and the sync/cancellable completion tick. A failed individual accessor read is swallowed to null so a successful operation's status query never turns into a throw.lib/sea/SeaOperationLifecycle.ts—seaFinished()merges the rich fields into the synthesized completion tick via a lazy thunk, sofinished({ callback })consumers see the same surface as a subsequentgetOperationStatus()call.lib/thrift-backend/wireSynthesis.ts—synthesizeThriftStatus()maps the rich fields intoTGetOperationStatusResp(numModifiedRowsre-boxed as anode-int64Int64to match the Thrift deserializer's wire shape;null→undefinedso the synthesized response matches the Thrift path, where an absent field is simply not set).numModifiedRowsnotenumModifiedRowsis currentlynullon SEA, by server behavior: the SEA/statementsREST API does not populatenum_modified_rowsin thestatusobject — verified live against pecotesting across dispositions/wait_timeout/user-agent. Instead, the server delivers a DML's modified-row count as result-set data (the result has columnsnum_affected_rows/num_inserted_rows). The kernel'sStatement.numModifiedRows()readsStatementStatus.num_modified_rows, which the server never sets, so it returnsnull.A companion kernel change (made in the single kernel PR) derives
num_modified_rowsfrom the DML result row. Once that lands, this driver wiring surfacesnumModifiedRowswith no further driver change — the title lists it as part of the operation-status surface this PR completes, not a value demonstrable today.The other three fields (
displayMessage,diagnosticInfo,errorDetailsJson) have legitimate server population paths (terminal-error states; admin-enabled workspaces forerrorDetailsJson) and are surfaced by this wiring where the server provides them.Testing
tests/unit/sea/operation-lifecycle.test.ts,tests/unit/sea/SeaOperationBackend.test.ts,tests/unit/sea/execution.test.ts, andtests/unit/thrift-backend/wireSynthesis.test.ts.useSEA: true):CREATE TABLE+INSERT INTO ... VALUES (1),(2),(3)+SELECT 1all execute and report status cleanly through the full chain;getOperationStatus()returns the rich-field shape (values reflect exactly what the kernel returns —numModifiedRowsnull today per the server note above), and the table is dropped after.This pull request and its description were written by Isaac.