Skip to content

Commit a2ce211

Browse files
committed
Revert broad terminal result-set behavior change
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
1 parent bf3a1b4 commit a2ce211

3 files changed

Lines changed: 10 additions & 35 deletions

File tree

‎lib/DBSQLOperation.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,14 @@ export default class DBSQLOperation implements IOperation {
147147

148148
private async fetchChunkInternal(options?: FetchOptions): Promise<Array<object>> {
149149
await this.failIfClosed();
150-
await this.waitUntilReadyThroughBackend(options);
151-
await this.failIfClosed();
152150

153151
if (!this.backend.hasResultSet()) {
154152
return [];
155153
}
156154

155+
await this.waitUntilReadyThroughBackend(options);
156+
await this.failIfClosed();
157+
157158
const defaultMaxRows = this.context.getConfig().fetchChunkDefaultMaxRows;
158159
const limit = options?.maxRows ?? defaultMaxRows;
159160
const result = await this.backend.fetchChunk({
@@ -246,9 +247,10 @@ export default class DBSQLOperation implements IOperation {
246247
return false;
247248
}
248249

249-
await this.waitUntilReadyThroughBackend();
250-
if (!this.backend.hasResultSet()) {
251-
return false;
250+
// Wait for operation to finish before checking for more rows
251+
// This ensures metadata can be fetched successfully
252+
if (this.backend.hasResultSet()) {
253+
await this.waitUntilReadyThroughBackend();
252254
}
253255

254256
// If we fetched all the data from server - check if there's anything buffered in result handler
@@ -259,12 +261,13 @@ export default class DBSQLOperation implements IOperation {
259261
public async getSchema(options?: GetSchemaOptions): Promise<TTableSchema | null> {
260262
return this.withErrorTelemetry(async () => {
261263
await this.failIfClosed();
262-
await this.waitUntilReadyThroughBackend(options);
263264

264265
if (!this.backend.hasResultSet()) {
265266
return null;
266267
}
267268

269+
await this.waitUntilReadyThroughBackend(options);
270+
268271
this.context.getLogger().log(LogLevel.debug, `Fetching schema for operation with id: ${this.id}`);
269272
if (this.backend instanceof ThriftOperationBackend) {
270273
const metadata = await this.backend.thriftResultMetadataResponse();

‎lib/thrift-backend/ThriftOperationBackend.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ export default class ThriftOperationBackend implements IOperationBackend {
144144
}
145145

146146
public hasResultSet(): boolean {
147-
return Boolean(this.operationHandle.hasResultSet);
147+
return Boolean(this.emptyResultOnNotFound || this.operationHandle.hasResultSet);
148148
}
149149

150150
public async fetchChunk({

‎tests/unit/thrift-backend/ThriftPrimaryKeysMetadata.test.ts‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -212,34 +212,6 @@ describe('Thrift getPrimaryKeys missing objects', () => {
212212
await expectEmptyPrimaryKeys(operation);
213213
});
214214

215-
for (const delivery of ['direct', 'poll']) {
216-
for (const firstCall of ['status', 'getSchema', 'fetchChunk', 'hasMoreRows'] as const) {
217-
it(`honors a terminal no-result flag via ${delivery} starting with ${firstCall}`, async () => {
218-
const context = createContext();
219-
const driver = sinon.spy(context.driver);
220-
const status = {
221-
status: { statusCode: TStatusCode.SUCCESS_STATUS },
222-
operationState: TOperationState.FINISHED_STATE,
223-
hasResultSet: false,
224-
};
225-
if (delivery === 'direct') {
226-
context.driver.getPrimaryKeysResp.directResults = { operationStatus: status };
227-
} else {
228-
context.driver.getOperationStatusResp = status;
229-
}
230-
const operation = await createSession(context).getPrimaryKeys(request);
231-
232-
await operation[firstCall]();
233-
234-
expect(await operation.getSchema()).to.be.null;
235-
expect(await operation.fetchAll()).to.deep.equal([]);
236-
expect(await operation.hasMoreRows()).to.be.false;
237-
expect(driver.getResultSetMetadata.called).to.be.false;
238-
expect(driver.fetchResults.called).to.be.false;
239-
});
240-
}
241-
}
242-
243215
it('preserves an unrelated wrapped RPC error without an operation state', async () => {
244216
const context = createContext();
245217
const status = {

0 commit comments

Comments
 (0)