From 625a7fe49981968633a30bb4554f65b5cd21db08 Mon Sep 17 00:00:00 2001 From: Vu Anh Phung Date: Fri, 2 Oct 2026 06:50:49 +0000 Subject: [PATCH 1/2] fix(metadata): accept null and empty primary-key namespace Signed-off-by: Vu Anh Phung --- lib/contracts/IDBSQLSession.ts | 4 ++-- lib/kernel/KernelSessionBackend.ts | 13 +------------ lib/thrift-backend/ThriftSessionBackend.ts | 4 ++-- native/kernel/index.d.ts | 2 +- tests/unit/DBSQLSession.test.ts | 15 +++++++++++++++ tests/unit/kernel/execution.test.ts | 22 ++++++++-------------- 6 files changed, 29 insertions(+), 31 deletions(-) diff --git a/lib/contracts/IDBSQLSession.ts b/lib/contracts/IDBSQLSession.ts index e7eb31d8..115a1618 100644 --- a/lib/contracts/IDBSQLSession.ts +++ b/lib/contracts/IDBSQLSession.ts @@ -122,8 +122,8 @@ export type FunctionsRequest = { }; export type PrimaryKeysRequest = { - catalogName?: string; - schemaName: string; + catalogName?: string | null; + schemaName?: string | null; tableName: string; /** * @deprecated This option is no longer supported and will be removed in future releases diff --git a/lib/kernel/KernelSessionBackend.ts b/lib/kernel/KernelSessionBackend.ts index 6cbc745e..685f5b8c 100644 --- a/lib/kernel/KernelSessionBackend.ts +++ b/lib/kernel/KernelSessionBackend.ts @@ -393,19 +393,8 @@ export default class KernelSessionBackend implements ISessionBackend { public async getPrimaryKeys(request: PrimaryKeysRequest): Promise { this.failIfClosed(); - // The kernel requires a catalog for primary-key lookup (`Identifier::new` - // rejects an empty string). The Thrift backend can forward an undefined - // catalog and let the server resolve a default; the kernel path cannot, - // so reject up front with a clear, actionable message rather than passing - // `''` and surfacing the kernel's opaque "identifier must not be empty". - if (request.catalogName === undefined || request.catalogName === '') { - throw new HiveDriverError( - 'kernel getPrimaryKeys requires a catalog — pass `catalogName` explicitly. (The Thrift backend ' + - 'can omit it and let the server resolve a default; the kernel kernel path requires it.)', - ); - } return this.runMetadata(() => - this.connection.getPrimaryKeys(request.catalogName as string, request.schemaName, request.tableName), + this.connection.getPrimaryKeys(request.catalogName, request.schemaName, request.tableName), ); } diff --git a/lib/thrift-backend/ThriftSessionBackend.ts b/lib/thrift-backend/ThriftSessionBackend.ts index ca9f8d90..76785964 100644 --- a/lib/thrift-backend/ThriftSessionBackend.ts +++ b/lib/thrift-backend/ThriftSessionBackend.ts @@ -302,8 +302,8 @@ export default class ThriftSessionBackend implements ISessionBackend { const driver = await this.context.getDriver(); const response = await driver.getPrimaryKeys({ sessionHandle: this.sessionHandle, - catalogName: request.catalogName, - schemaName: request.schemaName, + catalogName: request.catalogName ?? undefined, + schemaName: request.schemaName ?? undefined, tableName: request.tableName, runAsync: this.getRunAsyncForMetadataOperations(), ...getDirectResultsOptions(request.maxRows, this.context.getConfig()), diff --git a/native/kernel/index.d.ts b/native/kernel/index.d.ts index 51644c72..206f792c 100644 --- a/native/kernel/index.d.ts +++ b/native/kernel/index.d.ts @@ -403,7 +403,7 @@ export declare class Connection { * Primary keys for the given table. All three identifiers are * exact — ODBC `SQLPrimaryKeys` does not support patterns. */ - getPrimaryKeys(catalog: string, schema: string, table: string): Promise + getPrimaryKeys(catalog: string | undefined | null, schema: string | undefined | null, table: string): Promise /** * Foreign-key relationships. The foreign side must be fully * specified (catalog + schema + table); the parent side is diff --git a/tests/unit/DBSQLSession.test.ts b/tests/unit/DBSQLSession.test.ts index 59c19813..2a36083f 100644 --- a/tests/unit/DBSQLSession.test.ts +++ b/tests/unit/DBSQLSession.test.ts @@ -514,6 +514,21 @@ describe('DBSQLSession', () => { }); describe('getPrimaryKeys', () => { + it('should omit null namespace fields and preserve empty strings for Thrift', async () => { + for (const catalogName of [undefined, null, '', 'catalog']) { + for (const schemaName of [undefined, null, '', 'schema']) { + const context = new ClientContextStub(); + const session = createSessionForTest({ handle: sessionHandleStub, context }); + // eslint-disable-next-line no-await-in-loop + const result = await session.getPrimaryKeys({ catalogName, schemaName, tableName: 't1' }); + expect(result).instanceOf(DBSQLOperation); + expect(context.driver.getPrimaryKeysReq?.catalogName).to.equal(catalogName ?? undefined); + expect(context.driver.getPrimaryKeysReq?.schemaName).to.equal(schemaName ?? undefined); + expect(context.driver.getPrimaryKeysReq?.tableName).to.equal('t1'); + } + } + }); + it('should run operation', async () => { const session = createSessionForTest({ handle: sessionHandleStub, context: new ClientContextStub() }); const result = await session.getPrimaryKeys({ diff --git a/tests/unit/kernel/execution.test.ts b/tests/unit/kernel/execution.test.ts index af7d53c3..ca41feed 100644 --- a/tests/unit/kernel/execution.test.ts +++ b/tests/unit/kernel/execution.test.ts @@ -1094,25 +1094,19 @@ describe('KernelSessionBackend', () => { ]); }); - it('getPrimaryKeys rejects an omitted catalog up front (the kernel requires one)', async () => { + it('getPrimaryKeys forwards null, omitted, and empty namespace names unchanged', async () => { const connection = new FakeNativeConnection(); const session = makeSession(connection); - for (const request of [ - { schemaName: 'def', tableName: 't' }, - { catalogName: '', schemaName: 'def', tableName: 't' }, - ]) { - let thrown: unknown; - try { + const expected: unknown[][] = []; + for (const catalogName of [undefined, null, '', 'main']) { + for (const schemaName of [undefined, null, '', 'def']) { // eslint-disable-next-line no-await-in-loop - await session.getPrimaryKeys(request); - } catch (err) { - thrown = err; + const operation = await session.getPrimaryKeys({ catalogName, schemaName, tableName: 't' }); + expect(operation).to.be.instanceOf(KernelOperationBackend); + expected.push(['getPrimaryKeys', catalogName, schemaName, 't']); } - expect(thrown, `expected reject for ${JSON.stringify(request)}`).to.be.instanceOf(HiveDriverError); - expect((thrown as Error).message).to.match(/requires a catalog/); } - // The kernel call must NOT be reached (no empty-identifier sent over FFI). - expect(connection.metadataCalls.filter((c) => c[0] === 'getPrimaryKeys')).to.have.length(0); + expect(connection.metadataCalls).to.deep.equal(expected); }); it('getInfo synthesizes the three server-answered info types and rejects the rest', async () => { From 7f5dcc95ca437397ac1c815c7bb2ba5ff5a7376c Mon Sep 17 00:00:00 2001 From: Vu Anh Phung Date: Fri, 2 Oct 2026 07:15:37 +0000 Subject: [PATCH 2/2] fix(metadata): preserve public primary-key request contract Signed-off-by: Vu Anh Phung --- lib/contracts/IDBSQLSession.ts | 4 ++-- lib/thrift-backend/ThriftSessionBackend.ts | 4 ++-- tests/unit/DBSQLSession.test.ts | 13 +++++++++---- tests/unit/kernel/execution.test.ts | 7 ++++++- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/lib/contracts/IDBSQLSession.ts b/lib/contracts/IDBSQLSession.ts index 115a1618..e7eb31d8 100644 --- a/lib/contracts/IDBSQLSession.ts +++ b/lib/contracts/IDBSQLSession.ts @@ -122,8 +122,8 @@ export type FunctionsRequest = { }; export type PrimaryKeysRequest = { - catalogName?: string | null; - schemaName?: string | null; + catalogName?: string; + schemaName: string; tableName: string; /** * @deprecated This option is no longer supported and will be removed in future releases diff --git a/lib/thrift-backend/ThriftSessionBackend.ts b/lib/thrift-backend/ThriftSessionBackend.ts index 76785964..ca9f8d90 100644 --- a/lib/thrift-backend/ThriftSessionBackend.ts +++ b/lib/thrift-backend/ThriftSessionBackend.ts @@ -302,8 +302,8 @@ export default class ThriftSessionBackend implements ISessionBackend { const driver = await this.context.getDriver(); const response = await driver.getPrimaryKeys({ sessionHandle: this.sessionHandle, - catalogName: request.catalogName ?? undefined, - schemaName: request.schemaName ?? undefined, + catalogName: request.catalogName, + schemaName: request.schemaName, tableName: request.tableName, runAsync: this.getRunAsyncForMetadataOperations(), ...getDirectResultsOptions(request.maxRows, this.context.getConfig()), diff --git a/tests/unit/DBSQLSession.test.ts b/tests/unit/DBSQLSession.test.ts index 2a36083f..11f6142f 100644 --- a/tests/unit/DBSQLSession.test.ts +++ b/tests/unit/DBSQLSession.test.ts @@ -5,6 +5,7 @@ import DBSQLSession, { numberToInt64 } from '../../lib/DBSQLSession'; import InfoValue from '../../lib/dto/InfoValue'; import Status from '../../lib/dto/Status'; import DBSQLOperation from '../../lib/DBSQLOperation'; +import { PrimaryKeysRequest } from '../../lib/contracts/IDBSQLSession'; import ISessionBackend from '../../lib/contracts/ISessionBackend'; import ParameterError from '../../lib/errors/ParameterError'; import { TSessionHandle, TProtocolVersion } from '../../thrift/TCLIService_types'; @@ -514,16 +515,20 @@ describe('DBSQLSession', () => { }); describe('getPrimaryKeys', () => { - it('should omit null namespace fields and preserve empty strings for Thrift', async () => { + it('should forward null, omitted, and empty namespace names unchanged for Thrift', async () => { for (const catalogName of [undefined, null, '', 'catalog']) { for (const schemaName of [undefined, null, '', 'schema']) { const context = new ClientContextStub(); const session = createSessionForTest({ handle: sessionHandleStub, context }); // eslint-disable-next-line no-await-in-loop - const result = await session.getPrimaryKeys({ catalogName, schemaName, tableName: 't1' }); + const result = await session.getPrimaryKeys({ + catalogName, + schemaName, + tableName: 't1', + } as PrimaryKeysRequest); expect(result).instanceOf(DBSQLOperation); - expect(context.driver.getPrimaryKeysReq?.catalogName).to.equal(catalogName ?? undefined); - expect(context.driver.getPrimaryKeysReq?.schemaName).to.equal(schemaName ?? undefined); + expect(context.driver.getPrimaryKeysReq?.catalogName).to.equal(catalogName); + expect(context.driver.getPrimaryKeysReq?.schemaName).to.equal(schemaName); expect(context.driver.getPrimaryKeysReq?.tableName).to.equal('t1'); } } diff --git a/tests/unit/kernel/execution.test.ts b/tests/unit/kernel/execution.test.ts index ca41feed..e1d35f4f 100644 --- a/tests/unit/kernel/execution.test.ts +++ b/tests/unit/kernel/execution.test.ts @@ -21,6 +21,7 @@ import KernelSessionBackend from '../../../lib/kernel/KernelSessionBackend'; import KernelOperationBackend from '../../../lib/kernel/KernelOperationBackend'; import { KernelNativeBinding, KernelConnection, KernelStatement } from '../../../lib/kernel/KernelNativeLoader'; import IClientContext, { ClientConfig } from '../../../lib/contracts/IClientContext'; +import { PrimaryKeysRequest } from '../../../lib/contracts/IDBSQLSession'; import IDBSQLLogger, { LogLevel } from '../../../lib/contracts/IDBSQLLogger'; import HiveDriverError from '../../../lib/errors/HiveDriverError'; import ParameterError from '../../../lib/errors/ParameterError'; @@ -1101,7 +1102,11 @@ describe('KernelSessionBackend', () => { for (const catalogName of [undefined, null, '', 'main']) { for (const schemaName of [undefined, null, '', 'def']) { // eslint-disable-next-line no-await-in-loop - const operation = await session.getPrimaryKeys({ catalogName, schemaName, tableName: 't' }); + const operation = await session.getPrimaryKeys({ + catalogName, + schemaName, + tableName: 't', + } as PrimaryKeysRequest); expect(operation).to.be.instanceOf(KernelOperationBackend); expected.push(['getPrimaryKeys', catalogName, schemaName, 't']); }