[vuanhphung]

DDL such as CREATE TABLE and an empty query such as SELECT 'sample' AS Result WHERE 1 = 0 look the same: zero rows and, on Thrift, the same schema. Live probes show why. The server sets hasResultSet on every Thrift handle and reports a placeholder Result STRING column in TTableSchema for statements without columns (DDL, USE, CREATE VIEW). The Arrow schema it sends alongside is accurate: it has no fields for DDL, while the empty SELECT keeps its Result field. The kernel (SEA) returns a zero-column schema for DDL.

This adds an optional hasResultSet to getResultMetadata(). It is derived from the Arrow schema on Thrift and from the decoded schema on the kernel. An explicit hasResultSet: false from the Thrift server is honored. Catalog operations report true. On Thrift with arrowEnabled: false there is no Arrow schema, so it stays undefined rather than guessing. Existing behavior (getSchema(), the Thrift-shaped getMetadata(), fetch) is unchanged. DML reports true, since the server returns num_affected_rows.

Verification:

• New unit tests use Arrow schema bytes captured from a live warehouse.
• A new e2e spec, tests/e2e/result_set_presence.test.ts, passes live on Thrift and kernel. The related arrow, cloudfetch, primary-keys, staging and kernel smoke specs also pass (24 tests).
• npm test (1534 passing), lint, prettier, type-check and build are clean.

Performance: request counts are identical to main, 1 RPC per operation for every workload on Thrift. The fetch path doesn't touch the new code. On Thrift, getResultMetadata() decodes the Arrow schema once per operation (memoized): about 5 µs for a narrow schema (local micro-benchmark, medians of 3 alternating runs, main 0.2–0.4 µs vs branch 4.6–5.5 µs). The kernel reads fields.length on a schema it already decodes. Live medians in ms (main vs branch, 3 fresh processes each, alternating, 10 iterations):

Workload	Thrift (main → branch)	Kernel (main → branch)
SELECT + fetch + close	132.5 → 113.0	131.8 → 127.6
DDL + metadata + fetch + close	251.3 → 209.4	216.0 → 209.2

Fixes #309

This pull request and its description were written by Isaac.

[peco-review-bot]

Verdict: 1 Nit

Looks good, no blocking issues. The Thrift backend works out hasResultSet from the Arrow schema (decoded once and cached against the stable metadata buffer), always reports true for catalog operations, and leaves it undefined when there's no Arrow schema. The kernel backend works it out from the field count of a schema it already decodes. Both have unit tests built from captured schema bytes and an e2e spec, and the optional field doesn't change any existing behavior.

Other findings

• ⚪ Nit — In tests/e2e/result_set_presence.test.ts, the Arrow-disabled test calls openSession(false, { arrowEnabled: false }) before its try. If connecting fails, the getConfig stub is never restored by sinon.restore() (the stub is on a throwaway client, so the impact is negligible). Moving the call inside the try, or adding an afterEach(() => sinon.restore()), would make cleanup unconditional.

[vuanhphung]

Addressed the nit in f171c1c: the e2e suite now restores sinon stubs in afterEach, so the getConfig stub is cleaned up even if connecting fails.

[peco-review-bot]

Verdict: 1 Low

Looks good, with one low concern: on Thrift, the new hasResultSet ignores an explicit false that the server already reported on the handle. The Arrow-schema derivation, the kernel field-count check and the catalog/EmptyOperationBackend true paths are consistent with the documented contract. Unit and e2e tests cover these paths.

[peco-review-bot]

Verdict: 1 Nit

Looks good: no correctness concerns found. DBSQLOperation.getResultMetadata() waits for the operation to be ready before reading the handle's hasResultSet flag, so the flag is current. Catalog and primary-key fallback operations return true early, and the memoized Arrow decode is keyed on the buffer reference. Unit tests (with captured schema bytes) and e2e tests cover the DDL, empty-SELECT, catalog, Arrow-disabled and decode-failure cases.

Other findings

• ⚪ Nit — resultSetPresence drops Arrow schema decode failures without logging anything (catch { value = undefined; }). Logging the failure at LogLevel.debug would make an unexpected undefined easier to diagnose in the field. The contract still holds without it.

[vuanhphung]

Addressed the round-3 nit in 8381b44: a failed Arrow schema decode is now logged at debug level (operation id + error message), and the unit test asserts it.

[peco-review-bot]

✅ No issues identified by the review bot.