Commit 4ba2175
authored
[SEA-NodeJS] (2/3) SEA execution + result fetching (#410)
* feat(sea): SEA execution + result fetching [2/3]
Second of three stacked PRs (base: [1/3] connect + auth). Wires the
statement-execution + result-read path:
- SeaSessionBackend.executeStatement: real implementation — runs SQL via the
napi Connection and returns a SeaOperationBackend (replaces [1/3]'s stub).
- SeaOperationBackend: fetch pipeline (napi Statement.fetchNextBatch →
SeaResultsProvider → ArrowResultConverter → ResultSlicer) plus operation
cancel/close/finished via the SeaOperationLifecycle helpers.
- SeaResultsProvider / SeaArrowIpc / SeaArrowIpcDurationFix: Arrow IPC decode
for inline + CloudFetch result batches (the duration-fix pre-processor
rewrites Arrow Duration → Int64 so apache-arrow@13 can read it).
- ArrowResultConverter: constructor now takes the neutral { schema? } shape so
both the Thrift and SEA backends construct it without an adapter.
- flatbuffers pinned to 23.5.26 to match apache-arrow@13's nested copy.
Tests: executeStatement + openSession forwarding, M0 datatype round-trip
through the shared converter (primitives + ARRAY/MAP/STRUCT), multi-batch
streaming, and the neutral-metadata converter contract. Full INTERVAL-type
value parity + exhaustive operation-lifecycle coverage land in [3/3].
Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
* fix(sea): address #410 review — fetch cleanup, cooperative cancel, parity, docs
Validated each finding against a live pecotesting warehouse first; the headline
INTERVAL story turned out to be split-artifact, not breakage.
- F7: getResultMetadata stored the *unpatched* Duration IPC bytes in
meta.arrowSchema while advertising ArrowBased — store the patched bytes so an
ArrowBased consumer doesn't hit `Unrecognized type "Duration" (18)`.
- F3: fetchChunk now honors the `isClosed` cooperative-cancel probe (parity with
ThriftOperationBackend) at its yield points.
- F6: on a fetch error, best-effort close the statement (napi contract: stream
is unspecified after Err) and surface a typed kernel error via decodeNapiKernelError.
- F9: cancel-after-fetch now throws the canonical OperationStateError(Canceled)
("The operation was canceled by a client") — byte-matches the Thrift message.
- F10: typed HiveDriverError (not raw Error) in the schema/fetchNextBatch guards.
- F1: corrected SeaArrowIpcDurationFix docs — on this layer the rewriter only
makes Duration *decodable* (raw Int64); the duration_unit formatter lands in
#411 (verified live: byte-identical to Thrift).
- F5: documented that nested Duration is a SHARED apache-arrow@13 limitation —
verified the Thrift backend throws the identical error, so SEA matches parity.
- F2: added a live e2e that drives a real Arrow Duration column through the
rewriter (asserts no "Duration (18)" crash + raw-Int64 on this layer).
- F8: pinned the no-`Failed` invariant in status() (failures reject at submit).
- F12: renamed SeaResultsProvider's SeaStatementHandle → SeaFetchHandle (was a
name collision with the lifecycle interface of a different shape).
- F13: dropped the no-op await on the synchronous statement.schema().
- F14: fixed the Float-precision comment (Precision enum, not bit-width).
- F15: SeaResultsProvider.prime loops instead of self-recursing on empty batches.
Deferred (noted on the PR): F4 (per-batch triple-decode perf) and F11
(hasResultSet() hard-coded true for M0).
Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
* fix(sea): address #410 review — error fidelity, fetch perf, validation, tests
Addresses the 8 review threads on PR #410. Validated against a live
warehouse (results-e2e parity gate + interval-duration-e2e + execution-e2e
all pass) plus new/updated unit tests.
- SeaOperationLifecycle: rethrowKernelError now delegates to the canonical
decodeNapiKernelError, so cancel/close errors get the same fidelity as
fetch errors — the sqlState remap (envelope field is `sqlState`, the old
code read `sqlstate` and dropped it), the kernelMetadata namespace, and
the strict `startsWith` sentinel match (was a loose `indexOf >= 0`).
- SeaArrowIpc: replace decodeIpcBatch (full RecordBatchReader
materialization just to sum row counts) with countRowsInIpc, which reads
RecordBatch header `length` via MessageReader and skips bodies — no
vector decode. Removes ~2x Arrow decode CPU + transient allocation on the
fetch hot path (the converter still re-decodes for values). SeaResultsProvider
switched to it.
- SeaArrowIpc: hermetic unit tests (tests/unit/sea/SeaArrowIpc.test.ts) for
the framing walk, no-op/garbage rewrite paths, the row-count path, and the
empty-schema guard. (The Duration-positive rewrite stays covered by the
live e2e — apache-arrow@13 can't construct a Duration column hermetically.)
- SeaOperationBackend: on a fetch-error cleanup close() that also fails, log
the failure at warn (statement may leak) instead of fully swallowing it —
the original fetch error is still surfaced.
- SeaSessionBackend: reject queryTags / useLZ4Compression /
stagingAllowedLocalPath with M0-style errors instead of silently ignoring
them (+ unit tests). Silent no-ops are the worst failure mode for callers.
- SeaArrowIpc: throw a typed HiveDriverError (not a raw TypeError) when an
IPC payload carries no schema.
- SeaArrowIpcDurationFix.readMessageAt: fail-closed guards for negative
metadataLength / bodyLength (was relying on subarray clamping).
- Fix stale `tests/integration/sea/...` doc refs → `tests/e2e/sea/`.
Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
* test(telemetry): de-flake FeatureFlagCache placeholder test (network seam)
The "fetchFeatureFlag should return false as placeholder implementation"
test called the real fetchFeatureFlag, which makes an HTTP call via
fetchWithRetry (10s timeout) to a bogus host. Under mocha's 2s default it
only passed when the DNS failure happened to resolve quickly — flaky
across runners and Node versions (it timed out on Node 14/16/18 in CI,
fail-fast-canceling the rest of the matrix; Node 20 passed).
Stub the fetchWithRetry network seam so the test deterministically
exercises the behavior under test (fetchFeatureFlag resolves to false)
with no real network call. No production code change.
Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
---------
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>1 parent 623a140 commit 4ba2175
18 files changed
Lines changed: 2891 additions & 22 deletions
File tree
- lib
- result
- sea
- tests
- e2e/sea
- unit
- result
- sea
- telemetry
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | | - | |
| 16 | + | |
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| |||
42 | 42 | | |
43 | 43 | | |
44 | 44 | | |
45 | | - | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
46 | 51 | | |
47 | 52 | | |
48 | 53 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
0 commit comments