Skip to content

Commit 2b3f46b

Browse files
committed
Add opt-in catalog escaping for metadata requests
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
1 parent 3076abc commit 2b3f46b

12 files changed

Lines changed: 213 additions & 5 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
## Unreleased
44

5+
- Add `escapeMetadataCatalogName` (default `false`) to escape catalog underscores in Thrift and kernel metadata requests.
56
- Fix Thrift `getFunctions()` to report the requested `FUNCTION_CAT`, matching JDBC and kernel.
67

78
## 2.2.0

‎CONNECTION_PARAMETERS.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,23 @@ disabling verification.
104104
| `disableRowMaterialization` | `boolean` | ✅ | ✅ | `false` | Fetches and parses Arrow batches but returns `null` row placeholders instead of converting cells. Intended for fetch-throughput tests. |
105105
| `enableMetricViewMetadata` | `boolean` | ✅ | ⚠️ | `false` | Injected into session configuration on both paths. Kernel may drop its non-allowlisted configuration key. |
106106

107+
## Metadata catalog filters
108+
109+
| Option | Type | Thrift | Kernel | Default Value | Note |
110+
| --------------------------- | --------- | :----: | :----: | ------------- | ------------------------------------------------------------------------------------------------------------- |
111+
| `escapeMetadataCatalogName` | `boolean` | ✅ | ✅ | `false` | Escapes unescaped `_` in catalog filters; existing `\_` escapes and `%` wildcards are preserved, matching JDBC. |
112+
113+
Set `escapeMetadataCatalogName: true` in `client.connect(...)` to treat catalog
114+
underscores literally in metadata requests. For example, `comparator_tests`
115+
matches that catalog rather than also matching `comparatorXtests`. Schema,
116+
table, column, and function name patterns are unaffected.
117+
118+
Thrift applies this to `getSchemas`, `getTables`, `getColumns`, and
119+
`getFunctions`. Kernel applies it to `getTables`, avoiding catalog enumeration
120+
for names without `%` wildcards; its other methods already use exact catalog
121+
identifiers. `getPrimaryKeys` and `getCrossReference` use exact catalog
122+
identifiers on both backends.
123+
107124
## Session defaults (`openSession(request)`)
108125

109126
| Option | Type | Thrift | Kernel | Default Value | Note |

‎lib/DBSQLClient.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,8 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
157157

158158
useLZ4Compression: true,
159159

160+
escapeMetadataCatalogName: false,
161+
160162
preserveBigNumericPrecision: false,
161163

162164
disableRowMaterialization: false,
@@ -696,6 +698,10 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
696698
this.config.enableMetricViewMetadata = options.enableMetricViewMetadata;
697699
}
698700

701+
if (options.escapeMetadataCatalogName !== undefined) {
702+
this.config.escapeMetadataCatalogName = options.escapeMetadataCatalogName;
703+
}
704+
699705
// Opt-in: preserve DECIMAL (string) / BIGINT (bigint) precision in results.
700706
if (options.preserveBigNumericPrecision !== undefined) {
701707
this.config.preserveBigNumericPrecision = options.preserveBigNumericPrecision;

‎lib/contracts/IClientContext.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export interface ClientConfig {
2525

2626
useLZ4Compression: boolean;
2727
enableMetricViewMetadata?: boolean;
28+
escapeMetadataCatalogName?: boolean;
2829

2930
// When true, DECIMAL values are returned as exact strings and 64-bit
3031
// integers as JS `bigint`, instead of being coerced to a lossy `number`.

‎lib/contracts/IDBSQLClient.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,13 @@ export type ConnectionOptions = {
6868
proxy?: ProxyOptions;
6969
enableMetricViewMetadata?: boolean;
7070

71+
/**
72+
* Escape unescaped `_` in metadata catalog filters on both backends.
73+
* Existing `\_` escapes and `%` wildcards are preserved, matching JDBC.
74+
* Defaults to `false` to preserve the current metadata behavior.
75+
*/
76+
escapeMetadataCatalogName?: boolean;
77+
7178
/**
7279
* Verify the server's TLS certificate on the primary Thrift transport.
7380
* Secure-by-default: omitting this leaves full chain + hostname verification

‎lib/kernel/KernelSessionBackend.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import { v4 as uuidv4 } from 'uuid';
1616
import ISessionBackend from '../contracts/ISessionBackend';
17+
import escapeCatalogName from '../utils/escapeCatalogName';
1718
import IOperationBackend from '../contracts/IOperationBackend';
1819
import IClientContext from '../contracts/IClientContext';
1920
import {
@@ -367,8 +368,12 @@ export default class KernelSessionBackend implements ISessionBackend {
367368

368369
public async getTables(request: TablesRequest): Promise<IOperationBackend> {
369370
this.failIfClosed();
371+
// Other kernel metadata methods already take exact catalog identifiers.
372+
const catalogName = this.context.getConfig().escapeMetadataCatalogName
373+
? escapeCatalogName(request.catalogName)
374+
: request.catalogName;
370375
return this.runMetadata(() =>
371-
this.connection.listTables(request.catalogName, request.schemaName, request.tableName, request.tableTypes),
376+
this.connection.listTables(catalogName, request.schemaName, request.tableName, request.tableTypes),
372377
);
373378
}
374379

‎lib/thrift-backend/ThriftSessionBackend.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import ParameterError from '../errors/ParameterError';
3232
import { DBSQLParameter, DBSQLParameterValue } from '../DBSQLParameter';
3333
import { LogLevel } from '../contracts/IDBSQLLogger';
3434
import ThriftOperationBackend from './ThriftOperationBackend';
35+
import escapeCatalogName from '../utils/escapeCatalogName';
3536

3637
interface OperationResponseShape {
3738
status: TStatus;
@@ -151,6 +152,10 @@ export default class ThriftSessionBackend implements ISessionBackend {
151152
return ProtocolVersion.supportsAsyncMetadataOperations(this.serverProtocolVersion) ? true : undefined;
152153
}
153154

155+
private maybeEscapeCatalogName(catalogName: string | undefined): string | undefined {
156+
return this.context.getConfig().escapeMetadataCatalogName ? escapeCatalogName(catalogName) : catalogName;
157+
}
158+
154159
public get id(): string {
155160
const sessionId = this.sessionHandle?.sessionId?.guid;
156161
return sessionId ? stringify(sessionId) : NIL;
@@ -238,7 +243,7 @@ export default class ThriftSessionBackend implements ISessionBackend {
238243
const driver = await this.context.getDriver();
239244
const response = await driver.getSchemas({
240245
sessionHandle: this.sessionHandle,
241-
catalogName: request.catalogName,
246+
catalogName: this.maybeEscapeCatalogName(request.catalogName),
242247
schemaName: request.schemaName,
243248
runAsync: this.getRunAsyncForMetadataOperations(),
244249
...getDirectResultsOptions(request.maxRows, this.context.getConfig()),
@@ -250,7 +255,7 @@ export default class ThriftSessionBackend implements ISessionBackend {
250255
const driver = await this.context.getDriver();
251256
const response = await driver.getTables({
252257
sessionHandle: this.sessionHandle,
253-
catalogName: request.catalogName,
258+
catalogName: this.maybeEscapeCatalogName(request.catalogName),
254259
schemaName: request.schemaName,
255260
tableName: request.tableName,
256261
tableTypes: request.tableTypes,
@@ -274,7 +279,7 @@ export default class ThriftSessionBackend implements ISessionBackend {
274279
const driver = await this.context.getDriver();
275280
const response = await driver.getColumns({
276281
sessionHandle: this.sessionHandle,
277-
catalogName: request.catalogName,
282+
catalogName: this.maybeEscapeCatalogName(request.catalogName),
278283
schemaName: request.schemaName,
279284
tableName: request.tableName,
280285
columnName: request.columnName,
@@ -289,7 +294,7 @@ export default class ThriftSessionBackend implements ISessionBackend {
289294
const driver = await this.context.getDriver();
290295
const response = await driver.getFunctions({
291296
sessionHandle: this.sessionHandle,
292-
catalogName: request.catalogName,
297+
catalogName: this.maybeEscapeCatalogName(request.catalogName),
293298
schemaName: request.schemaName,
294299
functionName: request.functionName,
295300
runAsync: this.getRunAsyncForMetadataOperations(),

‎lib/utils/escapeCatalogName.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
/** Escape catalog underscores without double-escaping existing `\_` sequences. */
2+
export default function escapeCatalogName(catalogName: string | undefined): string | undefined {
3+
if (catalogName == null) {
4+
return catalogName;
5+
}
6+
return catalogName.replace(/\\?_/g, '\\_');
7+
}

‎tests/unit/DBSQLClient.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,28 @@ describe('DBSQLClient retry-policy ConnectionOptions', () => {
793793
});
794794
});
795795

796+
describe('DBSQLClient.escapeMetadataCatalogName', () => {
797+
beforeEach(() => {
798+
sinon.stub(FeatureFlagCache.prototype as any, 'fetchFeatureFlags').resolves(new Map());
799+
});
800+
afterEach(() => sinon.restore());
801+
802+
it('defaults to false and accepts opt-in and opt-out on connect', async () => {
803+
const client = new DBSQLClient();
804+
expect(client.getConfig().escapeMetadataCatalogName).to.be.false;
805+
806+
await client.connect({ ...connectOptions, telemetryEnabled: false });
807+
expect(client.getConfig().escapeMetadataCatalogName).to.be.false;
808+
809+
await client.connect({ ...connectOptions, telemetryEnabled: false, escapeMetadataCatalogName: true });
810+
expect(client.getConfig().escapeMetadataCatalogName).to.be.true;
811+
812+
await client.connect({ ...connectOptions, telemetryEnabled: false, escapeMetadataCatalogName: false });
813+
expect(client.getConfig().escapeMetadataCatalogName).to.be.false;
814+
await client.close();
815+
});
816+
});
817+
796818
describe('DBSQLClient.enableMetricViewMetadata', () => {
797819
it('should store enableMetricViewMetadata config when enabled', async () => {
798820
const client = new DBSQLClient();

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,6 +1095,56 @@ describe('KernelSessionBackend', () => {
10951095
]);
10961096
});
10971097

1098+
for (const enabled of [undefined, false, true]) {
1099+
for (const [catalogName, escaped] of [
1100+
[undefined, undefined],
1101+
['', ''],
1102+
['main', 'main'],
1103+
['comparator_tests', 'comparator\\_tests'],
1104+
['a\\_b_c', 'a\\_b\\_c'],
1105+
['a_b%c', 'a\\_b%c'],
1106+
['%', '%'],
1107+
]) {
1108+
it(`handles catalog ${JSON.stringify(catalogName)} with escapeMetadataCatalogName=${enabled}`, async () => {
1109+
const connection = new FakeNativeConnection();
1110+
const session = new KernelSessionBackend({
1111+
connection,
1112+
context: makeContext(undefined, { escapeMetadataCatalogName: enabled }),
1113+
});
1114+
const request = Object.freeze({
1115+
catalogName,
1116+
schemaName: 'schema_%',
1117+
tableName: 'table_%',
1118+
columnName: 'column_%',
1119+
functionName: 'function_%',
1120+
});
1121+
await session.getSchemas(request);
1122+
await session.getTables(request);
1123+
await session.getColumns(request);
1124+
await session.getFunctions(request);
1125+
await session.getPrimaryKeys(request);
1126+
await session.getCrossReference({
1127+
parentCatalogName: 'parent_catalog',
1128+
parentSchemaName: 's',
1129+
parentTableName: 't',
1130+
foreignCatalogName: 'foreign_catalog',
1131+
foreignSchemaName: 's',
1132+
foreignTableName: 'f',
1133+
});
1134+
1135+
expect(connection.metadataCalls).to.deep.equal([
1136+
['listSchemas', catalogName, 'schema_%'],
1137+
['listTables', enabled ? escaped : catalogName, 'schema_%', 'table_%', undefined],
1138+
['listColumns', catalogName, 'schema_%', 'table_%', 'column_%'],
1139+
['listFunctions', catalogName, 'schema_%', 'function_%'],
1140+
['getPrimaryKeys', catalogName, 'schema_%', 'table_%'],
1141+
['getCrossReference', 'parent_catalog', 's', 't', 'foreign_catalog', 's', 'f'],
1142+
]);
1143+
await session.close();
1144+
});
1145+
}
1146+
}
1147+
10981148
it('getPrimaryKeys forwards null, omitted, and empty namespace names unchanged', async () => {
10991149
const connection = new FakeNativeConnection();
11001150
const session = makeSession(connection);

0 commit comments

Comments
 (0)