Skip to content

Commit 625a7fe

Browse files
committed
fix(metadata): accept null and empty primary-key namespace
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
1 parent be677a6 commit 625a7fe

6 files changed

Lines changed: 29 additions & 31 deletions

File tree

‎lib/contracts/IDBSQLSession.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,8 @@ export type FunctionsRequest = {
122122
};
123123

124124
export type PrimaryKeysRequest = {
125-
catalogName?: string;
126-
schemaName: string;
125+
catalogName?: string | null;
126+
schemaName?: string | null;
127127
tableName: string;
128128
/**
129129
* @deprecated This option is no longer supported and will be removed in future releases

‎lib/kernel/KernelSessionBackend.ts‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -393,19 +393,8 @@ export default class KernelSessionBackend implements ISessionBackend {
393393

394394
public async getPrimaryKeys(request: PrimaryKeysRequest): Promise<IOperationBackend> {
395395
this.failIfClosed();
396-
// The kernel requires a catalog for primary-key lookup (`Identifier::new`
397-
// rejects an empty string). The Thrift backend can forward an undefined
398-
// catalog and let the server resolve a default; the kernel path cannot,
399-
// so reject up front with a clear, actionable message rather than passing
400-
// `''` and surfacing the kernel's opaque "identifier must not be empty".
401-
if (request.catalogName === undefined || request.catalogName === '') {
402-
throw new HiveDriverError(
403-
'kernel getPrimaryKeys requires a catalog — pass `catalogName` explicitly. (The Thrift backend ' +
404-
'can omit it and let the server resolve a default; the kernel kernel path requires it.)',
405-
);
406-
}
407396
return this.runMetadata(() =>
408-
this.connection.getPrimaryKeys(request.catalogName as string, request.schemaName, request.tableName),
397+
this.connection.getPrimaryKeys(request.catalogName, request.schemaName, request.tableName),
409398
);
410399
}
411400

‎lib/thrift-backend/ThriftSessionBackend.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,8 +302,8 @@ export default class ThriftSessionBackend implements ISessionBackend {
302302
const driver = await this.context.getDriver();
303303
const response = await driver.getPrimaryKeys({
304304
sessionHandle: this.sessionHandle,
305-
catalogName: request.catalogName,
306-
schemaName: request.schemaName,
305+
catalogName: request.catalogName ?? undefined,
306+
schemaName: request.schemaName ?? undefined,
307307
tableName: request.tableName,
308308
runAsync: this.getRunAsyncForMetadataOperations(),
309309
...getDirectResultsOptions(request.maxRows, this.context.getConfig()),

‎native/kernel/index.d.ts‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tests/unit/DBSQLSession.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,21 @@ describe('DBSQLSession', () => {
514514
});
515515

516516
describe('getPrimaryKeys', () => {
517+
it('should omit null namespace fields and preserve empty strings for Thrift', async () => {
518+
for (const catalogName of [undefined, null, '', 'catalog']) {
519+
for (const schemaName of [undefined, null, '', 'schema']) {
520+
const context = new ClientContextStub();
521+
const session = createSessionForTest({ handle: sessionHandleStub, context });
522+
// eslint-disable-next-line no-await-in-loop
523+
const result = await session.getPrimaryKeys({ catalogName, schemaName, tableName: 't1' });
524+
expect(result).instanceOf(DBSQLOperation);
525+
expect(context.driver.getPrimaryKeysReq?.catalogName).to.equal(catalogName ?? undefined);
526+
expect(context.driver.getPrimaryKeysReq?.schemaName).to.equal(schemaName ?? undefined);
527+
expect(context.driver.getPrimaryKeysReq?.tableName).to.equal('t1');
528+
}
529+
}
530+
});
531+
517532
it('should run operation', async () => {
518533
const session = createSessionForTest({ handle: sessionHandleStub, context: new ClientContextStub() });
519534
const result = await session.getPrimaryKeys({

‎tests/unit/kernel/execution.test.ts‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,25 +1094,19 @@ describe('KernelSessionBackend', () => {
10941094
]);
10951095
});
10961096

1097-
it('getPrimaryKeys rejects an omitted catalog up front (the kernel requires one)', async () => {
1097+
it('getPrimaryKeys forwards null, omitted, and empty namespace names unchanged', async () => {
10981098
const connection = new FakeNativeConnection();
10991099
const session = makeSession(connection);
1100-
for (const request of [
1101-
{ schemaName: 'def', tableName: 't' },
1102-
{ catalogName: '', schemaName: 'def', tableName: 't' },
1103-
]) {
1104-
let thrown: unknown;
1105-
try {
1100+
const expected: unknown[][] = [];
1101+
for (const catalogName of [undefined, null, '', 'main']) {
1102+
for (const schemaName of [undefined, null, '', 'def']) {
11061103
// eslint-disable-next-line no-await-in-loop
1107-
await session.getPrimaryKeys(request);
1108-
} catch (err) {
1109-
thrown = err;
1104+
const operation = await session.getPrimaryKeys({ catalogName, schemaName, tableName: 't' });
1105+
expect(operation).to.be.instanceOf(KernelOperationBackend);
1106+
expected.push(['getPrimaryKeys', catalogName, schemaName, 't']);
11101107
}
1111-
expect(thrown, `expected reject for ${JSON.stringify(request)}`).to.be.instanceOf(HiveDriverError);
1112-
expect((thrown as Error).message).to.match(/requires a catalog/);
11131108
}
1114-
// The kernel call must NOT be reached (no empty-identifier sent over FFI).
1115-
expect(connection.metadataCalls.filter((c) => c[0] === 'getPrimaryKeys')).to.have.length(0);
1109+
expect(connection.metadataCalls).to.deep.equal(expected);
11161110
});
11171111

11181112
it('getInfo synthesizes the three server-answered info types and rejects the rest', async () => {

0 commit comments

Comments
 (0)