Skip to content

Commit 4ccf0b2

Browse files
committed
[SEA-NodeJS] surface server statement_id as op.id on the sync path
On the sync (cancellable) execute path the operation id was a client UUID, because the server statement_id isn't known at construction — the kernel publishes it mid-`result()` once the initial execute round-trip returns. That left a cancelled/closed sync op untraceable against server/kernel logs (the async path already had the id from `submit`). `id` now prefers the server statement_id once known (captured from the resolved `Statement`, then the live canceller slot), falling back to the construction-time UUID until then. Updated the fake to model the real null-until-resolved `statementId` and assert op.id flips from UUID → server id after the execute completes. Validated live: SELECT 1 op.id is a UUID before fetch and the real `01f1…` statement id after. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
1 parent f028f73 commit 4ccf0b2

2 files changed

Lines changed: 30 additions & 4 deletions

File tree

lib/sea/SeaOperationBackend.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ export default class SeaOperationBackend implements IOperationBackend {
157157
// Undefined on the async / metadata paths.
158158
private readonly cancellableExecution?: SeaNativeCancellableExecution;
159159

160+
// Sync path: the server statement id captured from the resolved `Statement`
161+
// once `result()` settles. Undefined until then; surfaced via `id`.
162+
private resolvedStatementId?: string;
163+
160164
// Metadata path: terminal statement. Also the resolved fetch handle on the
161165
// sync-execute path once `cancellableExecution.result()` settles.
162166
private blockingStatement?: SeaOperationStatement;
@@ -231,7 +235,14 @@ export default class SeaOperationBackend implements IOperationBackend {
231235
}
232236

233237
public get id(): string {
234-
return this._id;
238+
// On the sync (cancellable) path the server statement id isn't known at
239+
// construction — it's published mid-`result()` once the initial execute
240+
// round-trip returns. Surface it once available (preferring the id captured
241+
// from the resolved `Statement`, then the live canceller slot) so a
242+
// cancelled/closed sync operation is traceable by the same id the server and
243+
// kernel logs key on, matching the async path (which has it from `submit`).
244+
// Falls back to the construction-time id (a client UUID) until then.
245+
return this.resolvedStatementId ?? this.cancellableExecution?.statementId ?? this._id;
235246
}
236247

237248
public hasResultSet(): boolean {
@@ -544,6 +555,9 @@ export default class SeaOperationBackend implements IOperationBackend {
544555
.result()
545556
.then((stmt) => {
546557
this.blockingStatement = stmt as unknown as SeaOperationStatement;
558+
// Capture the now-known server statement id (stable on the resolved
559+
// Statement) so `id` reports it for the rest of the lifecycle.
560+
this.resolvedStatementId = this.blockingStatement.statementId ?? this.resolvedStatementId;
547561
return stmt as unknown as SeaFetchHandle;
548562
})
549563
.catch((err) => {

tests/unit/sea/execution.test.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,10 @@ class FakeCancellableExecution {
137137

138138
public resultError: Error | null = null;
139139

140-
public readonly statementId = '01ef-fake-sync-id';
140+
// Mirrors the real `CancellableExecution.statementId`: `null` until the
141+
// initial execute round-trip publishes the server id mid-`result()`. The
142+
// resolved `Statement` (resultHandle) carries the id (`FakeNativeStatement`).
143+
public readonly statementId: string | null = null;
141144

142145
// When set, the result() promise stays pending until cancel() rejects it,
143146
// modelling a still-running blocking execute that a concurrent cancel aborts.
@@ -991,9 +994,18 @@ describe('SeaOperationBackend — sync (executeStatementCancellable) path', () =
991994
).to.throw(HiveDriverError, /exactly one/);
992995
});
993996

994-
it('id defaults to the cancellable execution statement id', () => {
997+
it('surfaces the resolved server statement id as op.id once the sync execute completes', async () => {
995998
const op = makeSyncOp(new FakeCancellableExecution());
996-
expect(op.id).to.equal('01ef-fake-sync-id');
999+
// Before result() resolves, the server id is not yet known (real
1000+
// `CancellableExecution.statementId` is null pre-round-trip), so op.id is the
1001+
// construction-time fallback — a client UUID, NOT the server statement id.
1002+
const idBefore = op.id;
1003+
expect(idBefore).to.be.a('string').and.have.length.greaterThan(0);
1004+
expect(idBefore).to.not.equal('01ef-fake-statement-id');
1005+
// Driving the blocking execute to terminal publishes the server id; op.id
1006+
// then reports it (parity with the async path, traceable in server logs).
1007+
await op.waitUntilReady();
1008+
expect(op.id).to.equal('01ef-fake-statement-id');
9971009
});
9981010

9991011
it('status() reports Running before result() and Succeeded after', async () => {

0 commit comments

Comments
 (0)