From 1986809695f7b961efea9560907048eceb9a68d4 Mon Sep 17 00:00:00 2001 From: Madhavendra Rathore Date: Fri, 29 May 2026 06:22:23 +0000 Subject: [PATCH 1/3] feat(thrift): per-statement metricViewMetadata knob on ExecuteStatementOptions Add `metricViewMetadata?: boolean` to `ExecuteStatementOptions`. When true, the Thrift backend forwards `spark.databricks.optimizer.enableMetricViewMetadata=true` via the request `confOverlay`, alongside any `query_tags` already serialized there. The option is per-statement only and does not persist across queries. The SEA backend will route the same key through napi `statementConf` once the kernel statement-options surface lands; until then the option is honored only on Thrift. Documented in the public option JSDoc so users do not silently lose the conf on SEA. Unit tests assert the option appears in the outgoing `TExecuteStatementReq` when set, is omitted when unset or `false`, and coexists with `queryTags` in the same `confOverlay`. Audit refs: rows 1.17 and 2.18 of sea-workflow/audits/2026-05-28-cross-driver-audit.md (F12 in the PR #347 audit). Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore --- lib/contracts/IDBSQLSession.ts | 11 ++++ lib/thrift-backend/ThriftSessionBackend.ts | 7 +++ tests/unit/DBSQLSession.test.ts | 58 ++++++++++++++++++++++ 3 files changed, 76 insertions(+) diff --git a/lib/contracts/IDBSQLSession.ts b/lib/contracts/IDBSQLSession.ts index 392f3108..d775f683 100644 --- a/lib/contracts/IDBSQLSession.ts +++ b/lib/contracts/IDBSQLSession.ts @@ -27,6 +27,17 @@ export type ExecuteStatementOptions = { * These tags apply only to this statement and do not persist across queries. */ queryTags?: Record; + /** + * Enable metric-view metadata expansion for this statement. When `true`, the + * Spark conf `spark.databricks.optimizer.enableMetricViewMetadata=true` is + * forwarded via the Thrift `confOverlay`. Applies to a single statement only; + * does not persist across queries. + * + * Equivalent SEA wiring will route the same key through napi `statementConf` + * once the kernel statement-options surface lands — until then this knob is + * honored only on the Thrift backend. + */ + metricViewMetadata?: boolean; }; export type TypeInfoRequest = { diff --git a/lib/thrift-backend/ThriftSessionBackend.ts b/lib/thrift-backend/ThriftSessionBackend.ts index c103ab4f..4a53a057 100644 --- a/lib/thrift-backend/ThriftSessionBackend.ts +++ b/lib/thrift-backend/ThriftSessionBackend.ts @@ -188,6 +188,13 @@ export default class ThriftSessionBackend implements ISessionBackend { request.confOverlay = { ...request.confOverlay, query_tags: serializedQueryTags }; } + if (options.metricViewMetadata === true) { + request.confOverlay = { + ...request.confOverlay, + 'spark.databricks.optimizer.enableMetricViewMetadata': 'true', + }; + } + if (ProtocolVersion.supportsCloudFetch(this.serverProtocolVersion)) { request.canDownloadResult = options.useCloudFetch ?? clientConfig.useCloudFetch; } diff --git a/tests/unit/DBSQLSession.test.ts b/tests/unit/DBSQLSession.test.ts index 51b27133..af7380e9 100644 --- a/tests/unit/DBSQLSession.test.ts +++ b/tests/unit/DBSQLSession.test.ts @@ -298,6 +298,64 @@ describe('DBSQLSession', () => { }); }); + describe('executeStatement with metricViewMetadata', () => { + const metricViewConfKey = 'spark.databricks.optimizer.enableMetricViewMetadata'; + + it('should forward the metric-view conf via confOverlay when metricViewMetadata is true', async () => { + const context = new ClientContextStub(); + const driver = sinon.spy(context.driver); + const session = new DBSQLSession({ handle: sessionHandleStub, context }); + + await session.executeStatement('SELECT * FROM my_metric_view', { metricViewMetadata: true }); + + expect(driver.executeStatement.callCount).to.eq(1); + const req = driver.executeStatement.firstCall.args[0]; + expect(req.confOverlay).to.deep.include({ [metricViewConfKey]: 'true' }); + }); + + it('should not set the metric-view conf when metricViewMetadata is omitted', async () => { + const context = new ClientContextStub(); + const driver = sinon.spy(context.driver); + const session = new DBSQLSession({ handle: sessionHandleStub, context }); + + await session.executeStatement('SELECT 1'); + + expect(driver.executeStatement.callCount).to.eq(1); + const req = driver.executeStatement.firstCall.args[0]; + expect(req.confOverlay?.[metricViewConfKey]).to.be.undefined; + }); + + it('should not set the metric-view conf when metricViewMetadata is false', async () => { + const context = new ClientContextStub(); + const driver = sinon.spy(context.driver); + const session = new DBSQLSession({ handle: sessionHandleStub, context }); + + await session.executeStatement('SELECT 1', { metricViewMetadata: false }); + + expect(driver.executeStatement.callCount).to.eq(1); + const req = driver.executeStatement.firstCall.args[0]; + expect(req.confOverlay?.[metricViewConfKey]).to.be.undefined; + }); + + it('should coexist with queryTags in the same confOverlay', async () => { + const context = new ClientContextStub(); + const driver = sinon.spy(context.driver); + const session = new DBSQLSession({ handle: sessionHandleStub, context }); + + await session.executeStatement('SELECT 1', { + metricViewMetadata: true, + queryTags: { team: 'eng' }, + }); + + expect(driver.executeStatement.callCount).to.eq(1); + const req = driver.executeStatement.firstCall.args[0]; + expect(req.confOverlay).to.deep.include({ + [metricViewConfKey]: 'true', + query_tags: 'team:eng', + }); + }); + }); + describe('getTypeInfo', () => { it('should run operation', async () => { const session = createSessionForTest({ handle: sessionHandleStub, context: new ClientContextStub() }); From fd81e67a1659dfcc1c9883456b5288a5698dda36 Mon Sep 17 00:00:00 2001 From: Madhavendra Rathore Date: Fri, 29 May 2026 06:44:16 +0000 Subject: [PATCH 2/3] =?UTF-8?q?docs(thrift):=20clarify=20metricViewMetadat?= =?UTF-8?q?a=20false=20=E2=89=A1=20omit=20in=20JSDoc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address DA review note N4 on PR #397: the public type is `boolean` (not `true`-literal), but the wiring treats `false` and omitted identically. Spell that out in the JSDoc so callers don't expect `false` to clear a server-side default. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore --- lib/contracts/IDBSQLSession.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/contracts/IDBSQLSession.ts b/lib/contracts/IDBSQLSession.ts index d775f683..08ebad26 100644 --- a/lib/contracts/IDBSQLSession.ts +++ b/lib/contracts/IDBSQLSession.ts @@ -33,6 +33,11 @@ export type ExecuteStatementOptions = { * forwarded via the Thrift `confOverlay`. Applies to a single statement only; * does not persist across queries. * + * Setting `false` is equivalent to omitting the option — both leave + * `confOverlay` untouched and the server's session default applies. The + * option is opt-in only; there is no way to explicitly clear a server + * default from the client. + * * Equivalent SEA wiring will route the same key through napi `statementConf` * once the kernel statement-options surface lands — until then this knob is * honored only on the Thrift backend. From 2e55194230afb7917c3d001e5d5568c30e129fc2 Mon Sep 17 00:00:00 2001 From: Madhavendra Rathore Date: Mon, 1 Jun 2026 01:19:33 +0000 Subject: [PATCH 3/3] test(thrift): adapt metricViewMetadata tests to post-abstraction DBSQLSession Rebasing onto main brought in the #378 backend-abstraction, which changed the DBSQLSession constructor from `{ handle, context }` to `{ backend }`. The metricViewMetadata tests still assert the right thing (the driver's confOverlay carries the metric-view conf key, now built in ThriftSessionBackend), so switch their construction to the `createSessionForTest({ handle, context })` helper the rest of the suite uses post-abstraction. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore --- tests/unit/DBSQLSession.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/DBSQLSession.test.ts b/tests/unit/DBSQLSession.test.ts index af7380e9..ffc3f551 100644 --- a/tests/unit/DBSQLSession.test.ts +++ b/tests/unit/DBSQLSession.test.ts @@ -304,7 +304,7 @@ describe('DBSQLSession', () => { it('should forward the metric-view conf via confOverlay when metricViewMetadata is true', async () => { const context = new ClientContextStub(); const driver = sinon.spy(context.driver); - const session = new DBSQLSession({ handle: sessionHandleStub, context }); + const session = createSessionForTest({ handle: sessionHandleStub, context }); await session.executeStatement('SELECT * FROM my_metric_view', { metricViewMetadata: true }); @@ -316,7 +316,7 @@ describe('DBSQLSession', () => { it('should not set the metric-view conf when metricViewMetadata is omitted', async () => { const context = new ClientContextStub(); const driver = sinon.spy(context.driver); - const session = new DBSQLSession({ handle: sessionHandleStub, context }); + const session = createSessionForTest({ handle: sessionHandleStub, context }); await session.executeStatement('SELECT 1'); @@ -328,7 +328,7 @@ describe('DBSQLSession', () => { it('should not set the metric-view conf when metricViewMetadata is false', async () => { const context = new ClientContextStub(); const driver = sinon.spy(context.driver); - const session = new DBSQLSession({ handle: sessionHandleStub, context }); + const session = createSessionForTest({ handle: sessionHandleStub, context }); await session.executeStatement('SELECT 1', { metricViewMetadata: false }); @@ -340,7 +340,7 @@ describe('DBSQLSession', () => { it('should coexist with queryTags in the same confOverlay', async () => { const context = new ClientContextStub(); const driver = sinon.spy(context.driver); - const session = new DBSQLSession({ handle: sessionHandleStub, context }); + const session = createSessionForTest({ handle: sessionHandleStub, context }); await session.executeStatement('SELECT 1', { metricViewMetadata: true,