Runs SQL against a Databricks SQL warehouse (over Thrift by default, or the REST Statement Execution API via protocol="sea") and hands the result back as Arrow (a Cursor shaped like databricks-sql-python's) or streaming NDJSON. See README.md for the user-facing API; this file is about working on the package.
rust/arrowbricks_core/-- the actual hot path: statement submit/poll, bounded-concurrency chunk fetch, thechunk_indexreorder buffer, Arrow-IPC decode/write, NDJSON encode, and named-parameter/token_provider/volume-file support, all exposed to Python via PyO3. Builds into this same wheel as the compiled submodulearrowbricks._core(see the rootpyproject.toml's[tool.maturin]--module-name = "arrowbricks._core",manifest-pathpointing back at this crate'sCargo.toml) -- not a separately published package, sopip install arrowbricksis the only install step. See its ownREADME.mdfor the crate-level design (reorder buffer, heartbeat primitives, etc.) andrust/arrowbricks_core/tests_py/for its own PyO3-level test suite (run explicitly by path, not auto-discovered by a barepytest).src/thrift.rsis the second backend (protocol="thrift", the default -- see its own design-invariant entry below) -- a hand-rolledTBinaryProtocolreader/writer plus the ~15 structs/7 RPCs needed to speak Databricks' HiveServer2-compatibleTCLIServicedirectly;protocol="sea"(the original REST-based Statement Execution API path) remains fully supported as an explicit opt-in.src/arrowbricks/client.py--DatabricksClient: a thin wrapper building a._core.Client(this package's compiled Rust submodule) and delegating every method to it. No httpx, no real Python-level logic left -- validates constructor args (auth, timeouts) the same way it always has, then hands off. No hardcoded catalog/schema, no cloud-SDK dependency.src/arrowbricks/_streaming.py--HEARTBEAT/QueryTimeout/await_with_heartbeat(pure Python, shared bycursor.py),write_ipc_stream/ReplayableArrowChunk(delegate to._core.write_ipc_stream/._core.read_ipc_stream-- always uncompressed, see below), andstream_query_json-- which delegates its submit/poll/fetch/reorder/decode/NDJSON-encode entirely to._core.Client.stream_ndjson_lines; this module only forwards already-formatted lines.ReplayableArrowChunkwas accidentally dropped from the public API in 1.0.0 (arro3-io, its read-side dependency, was eliminated with no replacement) and restored in 1.0.1 on top of the new._core.read_ipc_stream-- seetests/test_replayable_arrow_chunk.pyandrust/arrowbricks_core/tests_py/test_ipc_stream.pyfor regression coverage. If you touch eitherwrite_ipc_streamorReplayableArrowChunkagain, check both for a paired change -- they're meant to round-trip each other's output.src/arrowbricks/cursor.py--Connection/Cursor, the DB-API-ish surface (execute/execute_streamed,fetchone/fetchmany/fetchall,fetchall_arrow/fetchmany_arrow,fetchall_streamed/fetchall_arrow_streamed). Delegates directly to._core.Client/._core.ResultSetfor lazy, chunk-buffered fetching -- there's no Python-level reorder buffer anymore (that'srust/arrowbricks_core/src/pipeline.rs's job);Cursorjust adapts the Rust result to the same public shape as before the cutover.fetchone/fetchmany/fetchall(row materialization) are the one place arro3-core is still needed -- see "Dependencies" below.src/arrowbricks/_core.pyi+src/arrowbricks/py.typed-- PEP 561 type stubs for the compiled submodule (a compiled PyO3 extension has no type info of its own without them).tests/-- a real localhttp.server-based mock warehouse (tests/conftest.py'sMockServer/mock_warehouse/mock_volume_filesfixtures), not respx -- respx only patches httpx's transport, and can't intercept._core's ownreqwestrequests.chunk_bytes_builderbuilds real Arrow-IPC chunk bytes via arro3 (a dev-only dependency here), so tests exercise the actual Arrow IPC round trip.examples/--basic.py(static token),cursor_paging.py(fetchmany/fetchmany_arrow over a large result),fastapi_sse.py(streaming NDJSON as SSE),fastapi_sse_pivot.py(buffered fetchall_streamed with one combined heartbeat/timeout budget across the wait+download phases),fastapi_sse_validated.py(sqlglot-based SQL validation -- single-SELECT + fully-qualified-table-allowlist -- in front offastapi_sse.py's otherwise-unvalidatedsqlquery param),azure_auth.py(Azure ADtoken_providerviaazure-identity, kept out of core deps on purpose -- don't widenty check's scope to include it),oauth_m2m_auth.py(Databricks OAuth machine-to-machine client-credentials flow, thetoken_providerequivalent ofdatabricks-sql-connector'sauth_type="databricks-oauth"for service principals -- unlikeazure_auth.pythis needs zero extra install,urllib.requestonly, same "zero required dependencies" reasoning as the package itself).sqlglotis likewise example-only, not a real dependency.
arrowbricks has zero required runtime dependencies. arro3-core is optional (pip install arrowbricks[arro3]), needed only by Cursor.fetchone/fetchmany/fetchall's row materialization (cursor.py's _table_to_rows) -- fetchall_arrow/fetchmany_arrow, execute, stream_query_json, upload_volume_file/delete_volume_file, and Cursor.description all work with nothing installed.
Why arro3-core specifically, and why it can't be dropped without real new engineering: the compiled ._core submodule returns Arrow data via pyo3-arrow's Table type, which is fully self-contained for construction and the Arrow C Data Interface (__arrow_c_stream__) -- confirmed by testing with arro3 fully uninstalled. But Table.column() (needed to materialize native Python values row-by-row) is designed by pyo3-arrow to hand back the caller's own real arro3.core objects for further arro3-ecosystem interop, not a value it can produce itself -- so any code path that calls .column() needs arro3 installed. Cursor.description's schema access avoids this already (._core.ResultSet.schema() reads the decoded Arrow schema directly in Rust, converting to (name, type_name) string pairs itself, rather than going through Table.schema -- which has the same arro3-requiring design as .column()). Fully eliminating arro3-core would mean reimplementing Arrow-array-to-Python-native-value conversion in Rust ourselves (handling every type: ints, floats, decimals, timestamps-with-timezones, nested lists/structs, nulls) -- real, correctness-sensitive engineering, deliberately not undertaken so far. httpx and arro3-io are both fully eliminated (Rust does its own HTTP via reqwest and its own Arrow-IPC/NDJSON writing).
"Zero required runtime dependencies" is about Python packages a caller has to pip install -- the Rust crate itself has its own Cargo dependencies (arrow, reqwest, lz4_flex, ...) that compile straight into the wheel and need no separate install step at all; lz4_flex (pure Rust, no C toolchain) decompresses cloud-fetch chunk bytes, see the design invariant on result_compression below.
uv sync --all-extras # builds the Rust extension via maturin -- needs a Rust toolchain AND a C one (`ring`, this crate's TLS crypto backend, compiles its own hand-optimized assembly via a `cc` build-dependency -- see Cargo.toml's own comment on the `rustls`/`hyper-rustls` deps for why `ring` over the default `aws-lc-rs`; this is not obvious from the crate's own direct dependencies)
uv run pytest -q # Cursor-level suite (tests/)
uv run pytest rust/arrowbricks_core/tests_py -v # PyO3-level suite, not auto-discovered by the above
uv run ruff check .
uv run ty check src testscargo test --no-default-features (from rust/arrowbricks_core/) runs the crate's own unit/integration tests without building the PyO3 extension module at all -- faster than a full maturin build for iterating on Rust-only logic (reorder buffer, heartbeat primitives, NDJSON encoding).
One-time setup per clone: prek install (needs uv tool install prek first if not already on PATH).
-
No cloud-SDK dependency. Auth is
token: strortoken_provider: Callable[[], str | Awaitable[str]]. Do not addazure-identity/boto3/etc. as a real dependency -- that belongs in the caller's app. -
No hardcoded catalog/schema.
catalog/schemadefault toNoneeverywhere. This package has zero knowledge of any specific Databricks workspace's naming. -
Zero required runtime dependencies -- keep it that way. See "Dependencies" above. Don't casually add a new hard dependency (Python or Rust-crate-that-drags-in-a-Python-requirement) without checking whether the Rust core can do it instead.
-
Chunk order is not fetch order, and a chunk_index is not guaranteed unique or contiguous.
rust/arrowbricks_core/src/pipeline.rs'sReorderBuffer(aHashMap<i64, VecDeque<ChunkItem>>, not a single chunk per index) is the one place this is handled now -- bothCursor's fetch methods andstream_query_jsongo through it. If you touch it, keep a test proving order survives out-of-order arrival AND that duplicate/missing indices never lose rows (seepipeline.rs's own#[cfg(test)]module, plustests/test_cursor.py::test_fetchall_preserves_order_despite_out_of_order_chunksandtests/test_streaming.py::test_stream_query_json_survives_duplicate_index_and_a_gap). -
Chunks are fetched lazily, not all upfront.
Cursor's fetch methods only pull as many chunks as afetchone/fetchmany/fetchallactually needs -- seeResultStream::fetch_at_leastinpipeline.rs. Don't "simplify" this into draining the whole result insideexecute(). -
execute_streamed's heartbeat/timeout only covers the wait for the statement to become ready -- not downloading any chunk. That was a real bug in this package's first release: a caller doingexecute_streamed()thenfetchall()got zero timeout enforcement and zero heartbeats during a slow multi-chunk download, exactly the case heartbeats exist for.fetchall_streamed/fetchall_arrow_streamedcover that second phase -- a caller wanting one combined budget across both phases must track its own deadline and pass the remaining time into the second call (seeexamples/fastapi_sse_pivot.py), since two independently-clockedtotal_timeout_ss would let a pathological case run up to 2x the intended ceiling.stream_query_jsonis different again -- its own submit/poll wait is never heartbeat-wrapped at all (only the chunk-by-chunk download is), matching its pre-cutover behavior; this is a preserved quirk, not something to "fix" casually. -
No silent row caps. There's no
ABSOLUTE_ROW_LIMIT-style ceiling baked in. If a caller wants one, that'srow_limit, which they pass explicitly. -
No retry dependency.
rust/arrowbricks_core/src/client.rs'sretry_callis a small hand-rolled exponential-backoff loop, not tenacity/etc -- the only retry pattern in the crate, so a dependency for it wasn't worth it. -
decode_chunk(pipeline.rs) decodes each chunk viaarrow_ipc::reader::StreamDecoderfed by anarrow::buffer::Bufferbuilt from the chunk'sbytes::Bytes, not the higher-levelStreamReaderover anIoCursor.Buffer::from(bytes::Bytes)is genuinely zero-copy (confirmed in arrow-buffer's own source:bytes.rs'simpl From<bytes::Bytes> for Bytesstores the originalbytes::BytesviaDeallocation::Custom, no memcpy), so for properly-aligned IPC data (the normal case -- Databricks writes it) decoded batches slice directly into the same allocation the network fetch already produced, instead ofStreamReadercopying every column out of the slice into freshly allocated buffers on every decode. Measured in isolation (not network-bound): ~6.16ms -> ~4.03ms per chunk decode on a 120-column/50k-row batch shaped like this session's own real-table benchmark (~35% faster decode) -- seepipeline.rs'sdecode_chunk_speed_vs_stream_reader(#[ignore]d, run manually viacargo test --release -- --ignored --nocapture decode_chunk_speed, since relative timing is too flaky for CI). This is a small piece of the real 5.6M-row/120-column table's ~90s total (network fetch dominates, not decode), but a real, free win with no correctness trade-off.StreamDecoder::decodeonly returns oneRecordBatchper call, unlikeStreamReader'sIterator--decode_chunkloops it until the buffer is drained; seedecode_chunk_reads_every_record_batch_in_a_multi_batch_streamfor the regression test (a single-batch-per-chunk bug here would pass every other test in the suite, since none of them writes more than one batch per chunk). Verified against a real workspace across every type this crate has already tested (NaN/Infinity, nested ARRAY/MAP/STRUCT, VARIANT-as-JSON-string, NULLs) after this swap, not just the synthetic fixtures.- A zero-length chunk blob is rejected explicitly, before it ever reaches
StreamDecoder. Found in an independent code review of theStreamDecoderswap above: an empty buffer makes thewhile !buffer.is_empty()loop a no-op, anddecoder.finish()then sees a still-pristine decoder state -- which its ownOk(())arm treats as a legitimately clean, empty stream. That meansdecode_chunkwould have silently returned zero batches with no error at all for a genuinely empty/corrupt chunk -- the same silent-truncation shape as the real multi-frame LZ4 bug this crate already shipped once (see theresult_compressionentry above). The oldStreamReader-based version failed loudly on the same input ("Expected schema message, found empty stream");decode_chunknow checksblob.is_empty()up front and errors instead of ever constructing a decoder. Seedecode_chunk_rejects_an_empty_blob(empty must error) and its companiondecode_chunk_accepts_a_schema_only_stream_with_zero_batches(a non-empty, schema-only, zero-RecordBatchstream -- a real, legitimate shape for an empty query result -- must still succeed, proving the empty check doesn't overcorrect). - Trailing bytes after a stream's own EOS marker now hard-error, where the old
StreamReadersilently ignored them. Also found in that same review. This is a deliberate, accepted behavior change (erroring beats silently dropping whatever came after a truncation point), not a bug -- real Databricks chunks have not been observed to have trailing bytes. Seedecode_chunk_errors_on_trailing_bytes_after_a_complete_stream. - The zero-copy claim has one honest exception, not mentioned in the first version of this entry: if a
RecordBatchmessage itself declared IPC buffer-level compression (a different, unrelated feature from this crate's own cloud-fetchresult_compressionunwrap, which already ran beforedecode_chunkever sees the bytes),arrow-ipc's own reader always decompresses into fresh buffers for that message regardless of alignment. Not something Databricks has been observed to use in this format, but not something this crate controls either --decode_chunk's own doc comment states this plainly rather than overclaiming universal zero-copy.
- A zero-length chunk blob is rejected explicitly, before it ever reaches
-
write_ipc_stream(and everything built on it) always writes uncompressed Arrow-IPC bodies.arrow-rs'sStreamWriter, no compression codec configured, ever. A compressed body (arro3's own default iscompression="LZ4") is transparently decompressed by some Arrow readers (e.g. DuckDB's) but not all --duckdb-wasm's browser-side decoder silently fails to parse it (this was a real bug in duckbricks 0.3.0, fixed in 0.3.1 -- see its CHANGELOG/git history). Never add compression here -- this invariant is about bytes we hand to a caller (Python or over the wire to something like duckdb-wasm), and is unrelated to the next point. -
Statement submission requests
result_compression: "LZ4_FRAME"by default (cloud-fetch transport compression) -- this is not the invariant above.execute_statementinclient.rsasks Databricks to LZ4-compress each chunk's external-link file (matchingdatabricks-sql-python's own default,enable_query_result_lz4_compression=True) unlesscompress_results=Falsewas passed toDatabricksClient/connect()/._core.Client(a runtime toggle,DbClient.with_compress_results/PyDbClient::new'scompress_resultskwarg -- not a compile-time constant, so a caller can rule it out without rebuilding) -- less data over the wire for exactly the case that dominates real-world latency: measured ~2x faster chunk-fetch time against a real 120-column/100k-row Databricks table (compressed ~3.6-4.7s vs uncompressed ~8.2-9.9s fetch time, same query, same warehouse).fetch_link_bytesdecompresses each chunk immediately after download (vialz4_flex::frame::FrameDecoder, gated on the manifest's ownresult_compressionfield actually confirming it -- never assumed just because we asked), before the bytes are Arrow-IPC-decoded or handed anywhere -- by the time any caller (Python or the write path above) sees a chunk's bytes, they're already the plain uncompressed Arrow-IPC/JSON this crate has always produced.- A real chunk's compression is multiple LZ4 frames concatenated, not one. Verified against a live workspace: a single 50k-row/~1MB chunk came back as 18 separate frames.
FrameDecoder::read_to_endonly reads until the first frame's own end marker -- calling it once silently decoded just that first frame (~200 bytes, the Arrow schema message, no row data) with no error at all, which the pipeline then happily treated as a valid empty (0 rows, 0 columns) result. This shipped once because it was only ever tested against synthetic single-frame data; caught by testing against a real warehouse before a release, not by any test that existed at the time.decompress_lz4_framenow loopsread_to_endon a single, reusedFrameDecoderuntil its output stops growing -- aFrameDecoderresets its own frame state after eachEndMarkand picks up the next concatenated frame on a subsequent call against the same instance, so this needs no per-frame reconstruction. Also runs onspawn_blocking(matchingpipeline.rs's decode stage) rather than inline on the async task -- if you touch it, keepclient.rs's own multi-frame unit test andtests/wiremock_pipeline.rs's integration test (which compresses its mock data as several small concatenated frames specifically to exercise this, not just one). - See
tests/wiremock_pipeline.rs'sexecute_statement_requests_lz4_frame_compression/execute_statement_omits_compression_when_disabledandcompressed_pipeline_decompresses_lz4_frame_chunksfor the request-body-toggle and round-trip proof, respectively.
- A real chunk's compression is multiple LZ4 frames concatenated, not one. Verified against a live workspace: a single 50k-row/~1MB chunk came back as 18 separate frames.
-
chunk_fetch_concurrencydefaults to 64, not 32 -- and a 2026-08-09 attempt to bump it to 96 was reverted after being caught as a benchmarking false positive. The 32->64 bump's own justifying comment was based on an ad hoc mocked benchmark, explicitly flagged in the code as "not reproducible from a script in this repo." Measured directly against a real 400-chunk/5.6M-row/120-column table, repeated runs: 16=140s, 32=~113s, 64=114s, 96=~102s, 128=122s -- 16 is clearly worse, 128 clearly regresses, and 32-96 land in the same rough band with 96 nominally fastest but only ~10% ahead of 32 (workload/warehouse-shaped, not a fixed constant). 64 was picked as a safe middle-ground bump with no observed downside on real data, not a claim that it's the true optimum -- seeclient.rs'sDbClient::with_token_providerfor the exact numbers.- The false positive, for the record: re-attempted 2026-08-09 after switching away from
http2/aws-lc-rstoring(both could plausibly move the optimum). A first pass reported 96 as ~13-16% faster than 64 on bothbenchmark_tableand a second, much larger table (large_benchmark_table, 20M rows/295 cols) -- wrong, caught on review before shipping. The benchmark script calledconnect()/arrowbricks.connect()without ever passingchunk_fetch_concurrency=explicitly, so every "level" it claimed to test actually ran at whateverPyDbClient::new's own#[pyo3(signature = ...)]default was (unchanged at 64 throughout the whole test sequence, since only the Rust-sideDEFAULT_CHUNK_FETCH_CONCURRENCYconst had been edited to 96, and that constant is unconditionally overridden byPyDbClient::new's explicit.with_concurrency(chunk_fetch_concurrency)call for every real Python caller regardless of the constant's own value) -- i.e. every "64 vs 96 vs 128" comparison in that first pass was actually 64 vs 64 vs 64, and the reported gap was warehouse/network run-to-run noise, not a code effect. Caught by re-running a controlled, interleaved A/B (chunk_fetch_concurrency=passed explicitly each time -- a pure runtime parameter, no rebuild needed to test different values) onbenchmark_table: 64/96/64/96 measured 125.60s/129.92s/137.50s/134.30s -- no consistent winner, well within run-to-run noise. Reverted to 64 across all four places it's hardcoded (below). - This default is independently hardcoded in four places that must move together -- confirmed the hard way by the false positive above, which only updated one of them and so silently no-opped for every real Python caller:
client.rs's ownDEFAULT_CHUNK_FETCH_CONCURRENCYconst (read byDbClient::new/with_token_provideranddownload_slots's initial sizing, for a bare-Rust caller that never calls.with_concurrency()explicitly -- not what a Python caller ever goes through),lib.rs'sPyDbClient::new#[pyo3(signature = ...)]default (the one that actually matters for every Python caller, since.with_concurrency(...)there is unconditional), Python'sclient.py's own kwarg default plus_core.pyi's matching stub, andrust/arrowbricks_core/README.md's own API reference. Lesson for next time: a concurrency-sweep benchmark script must pass the parameter under test explicitly on every run, never rely on "I changed the default and rebuilt" -- those are two different, easily-conflated claims, and only one of them is what a real caller experiences. - Second instance of this exact threading pattern:
retry_attempts/retry_max_wait_s(2026-08-11). Same four-places shape (client.rs'sRETRY_ATTEMPTS/RETRY_MAX_WAIT_Sconsts -- now only the defaults forDbClient's ownretry_attempts/retry_max_wait_sfields, read byretry_call/retry_call_tracked, both promoted from free functions to&selfmethods so they can read per-client policy instead of a compile-time constant --lib.rs'sPyDbClient::newsignature defaults,client.py's kwarg defaults,_core.pyi's stub) avoided this entry's own false-positive pattern by copying its lesson directly: the runtime-override test (tests/test_retry.py) passesretry_attempts=/retry_max_wait_s=explicitly on every case, including the one proving the default is 6 (which still overridesretry_max_wait_sto a tiny value, so the test isn't also paying the real ~20s-per-attempt backoff cost) -- never relying on "the Rust constant says 6, so Python must see 6."- A different, real bug still shipped in the first draft, caught in independent code review, not by that test suite:
retry_attemptswas originally typedu32inPyDbClient::new's#[pyo3(signature = ...)], soDatabricksClient(..., retry_attempts=-1)failed PyO3's own argument conversion withOverflowErrorbefore this constructor's ownif retry_attempts < 1 { return Err(PyValueError::new_err(...)) }check ever ran -- contradicting theValueError-for-bad-retry-config contract this same session had just documented inclient.py/README.md/CHANGELOG.md (OverflowErrorisn't aValueErrorsubclass, soexcept ValueErrorwritten against that contract wouldn't catch it). No test had ever tried a negativeretry_attempts--test_retry_attempts_zero_rejectedonly covered0, which au32parameter accepts fine (PyO3 converts it before any validation runs) and only then rejects via the hand-written check, so the wrong-exception-type gap for negative values went unexercised. Fixed by typing the parameteri64(any value a caller could plausibly pass, negative included) and converting tou32viau32::try_from(not a bareascast, which would have silently wrapped a value larger thanu32::MAXinto some unrelated smallu32instead of erroring -- the identical "wrong result instead of an error" shape this whole fix exists to close) only after the< 1check confirms the value is in range. Seetests/test_retry.py'stest_retry_attempts_negative_rejected_with_value_error_not_overflow_error. Lesson for next time: a "reject bad input with a specific exception type" fix needs a test for every input shape that could plausibly reach the wrong exception path first (here: negative, not just zero) -- boundary-testing only the documented invalid value (0) missed the one that actually broke.
- A different, real bug still shipped in the first draft, caught in independent code review, not by that test suite:
- The false positive, for the record: re-attempted 2026-08-09 after switching away from
-
A statement's SUCCEEDED submit/poll response can already embed some chunks' presigned links directly (
result.external_links) -- use them instead of resolving viaGET .../result/chunks/{i}when present. Confirmed against a real workspace: a SUCCEEDED response's top-levelresult.external_linksalready contained chunk 0's URL, same EXTERNAL_LINKS disposition as always -- not conditional on any special request field.execute_statementinclient.rscaptures these intoChunkMeta::pre_resolved_links: Vec<String>(not a singleOption<String>-- achunk_indexcan carry more than one blob, same reasonfetch_chunk_indexreturnsVec<Bytes>andReorderBufferkeys onVecDeque; collapsing to one silently drops every link but the last for a duplicated index, an early draft of this feature had exactly that bug, caught in code review before it shipped). The fetch worker infetch_chunks_with_backpressurecalls the newfetch_pre_resolved_linksdirectly when non-empty, skipping that chunk's own resolution GET entirely.ResultLinkBody's fields are both#[serde(default)](not required) -- an omitempty-style server serializer could drop a zero-valuedchunk_index(exactly chunk 0, the case this optimization targets most) or an emptyexternal_link, and sinceauthed_jsonfails the wholeStatementResponseBodyparse on any missing required field with no retry, a required field here would turn an optional fast path into a fatal error for the entire statement; entries with an emptyexternal_linkare filtered out rather than trusted. Measured against the real warehouse: median 0.543s (resolve-then-fetch) vs 0.222s (embedded link straight to fetch) for one chunk -- a full round trip saved, real and consistent, not warehouse jitter (isolated by timing just the resolution+fetch step directly over raw HTTP, not the whole query). Only chunks whose links happen to already be embedded skip the GET -- for a large multi-chunk result that's typically just chunk 0, so the win is proportionally bigger for small/fast queries than for a huge multi-chunk fetch. Seetests/wiremock_pipeline.rs'spre_resolved_chunk0_link_skips_the_extra_resolution_get(asserts the resolution mock is hit exactly 0 times via wiremock's.expect(0), not just that the pipeline still produces correct rows some other way). -
prefer_inline(an opt-in kwarg onClient.execute/Cursor.execute/execute_streamed, defaultFalse) submits withdisposition: "INLINE", format: "JSON_ARRAY"instead of the normalEXTERNAL_LINKSpath, for a caller who already expects a small result and wants to skip the chunk-resolve-then-fetch round trip entirely. Two faster-looking alternatives were tried first and confirmed dead ends by direct testing against a real workspace, not documentation (which was unhelpful/unavailable):disposition: "INLINE_OR_EXTERNAL_LINKS"(HYBRID) returns HTTP 400"INLINE_OR_EXTERNAL_LINKS is not a supported disposition."on this workspace;disposition: "INLINE"withformat: "ARROW_STREAM"returns HTTP 400"Incompatible parameters: The format field must be JSON_ARRAY when the disposition field is INLINE."-- INLINE only ever comes back as JSON, never Arrow, so a JSON-row-to-Arrow-array conversion is unavoidable if this path is used at all.- INLINE's byte cap is exactly 26,214,400 bytes (25 MiB) and failure is a clean, typed error, never silent truncation -- confirmed by running the real 5.6M-row/120-column
benchmark_tabletable underdisposition=INLINE:FAILEDstate,error_code: BAD_REQUEST, message"Inline byte limit exceeded. Statements executed with disposition=INLINE can have a result size of at most 26214400 bytes. Please execute the statement with disposition=EXTERNAL_LINKS if you want to download the full result."This is exactly whatclient.rs'sexecute_arrow_statement_prefer_inlinepattern-matches on to trigger its fallback -- verified safe to build on before any conversion code was written. - A column type that
json_convert.rscannot convert produces an error after SUCCEEDED, without resubmitting SQL.json_array_to_record_batch/build_columncover scalar types and supported STRUCT fields. Unsupported types, including empty STRUCT arrays, returnErr;pipeline.rs'sexecute_lazy_prefer_inlinereports anArrowbricksErrornaming the succeeded statement. The caller decides whether another execution is safe. Only the recognized server-side INLINE byte-limit failure takes the automatic external-links fallback. - STRUCT columns of scalar fields are supported via the manifest's
type_text, nottype_namealone --type_namefor a STRUCT column is just the string"STRUCT", with no field information;type_textcarries a full recursive SQL DDL rendering (confirmed against a real workspace:"STRUCT<content: BINARY, thumbnail: BINARY>", or"STRUCT<a: TINYINT NOT NULL, ...>"with" NOT NULL"suffixes on some fields).json_convert.rs'sparse_struct_fields/parse_one_fieldtokenize this (tracking</(depth so a nested composite field or aDECIMAL(p,s)'s own comma isn't mistaken for a field separator), thenbuild_column's"STRUCT"arm recursively calls itself per field. Two things confirmed against a real workspace, not assumed: (1) a STRUCT's SQL DDL field-type spelling differs from this same manifest's own top-leveltype_namevocabulary for exactly three widths (TINYINT/SMALLINT/BIGINTvs.BYTE/SHORT/LONG--parse_one_fieldremaps these three; everything else matchesbuild_column's scalar arms unchanged); (2) a STRUCT value's JSON object key order does not match the field's declared order intype_text(e.g.{"e":"1.5","j":"2026-01-01","f":"2.5","a":"1",...}for fields declareda,b,c,d,e,...), so fields are looked up by name, never assumed positional. A nested composite field (STRUCT-of-STRUCT/ARRAY/MAP) is deliberately left unmapped -- its raw type text just falls through tobuild_column's catch-allErr, the same conversion error as any other unsupported type, rather than this parser attempting unbounded recursion. - Empirically, on this real workspace,
prefer_inlinedoes not actually beat the normalEXTERNAL_LINKSpath -- confirmed by a repeated, steady-state benchmark, not assumed from the theory that motivated building it. Averaged over 9 warm runs (one connection reused, first run discarded),prefer_inline=Truewas consistently a bit slower thanprefer_inline=Falseacross every shape tried againstbenchmark_table: 1000 rows/120 cols (920ms vs 629ms), 5/10/50 rows/120 cols (594-608ms vs 507-530ms), and a narrow 4-column/10-row query (519ms vs 474ms) -- never faster in any of these. Most likely cause: the round trip this feature was built to skip (the separate chunk-resolveGET) is often already skipped for chunk 0 by the pre-resolved-links optimization above, so there's little round trip left to save, while JSON_ARRAY is a more verbose wire format than compressed Arrow-IPC and per-cell string parsing (decimal/timestamp/base64/STRUCT-field-lookup) is real CPU work the native IPC decoder doesn't pay. Kept as a correct, tested, opt-in feature anyway (it may still win in a higher-latency environment, or against a warehouse/table shape where the chunk-resolve round trip genuinely isn't pre-resolved) -- but do not assume it closes this package's remaining gap withdatabricks-sql-connectoron small queries; it measurably doesn't, at least not here. That gap's actual source, investigated and closed separately: see the SEA session pool entry below --databricks-sql-connector's own SEA mode (use_sea=True) was consistently faster than this crate's session-less SEA submissions for an identical query, which turned out to be about session reuse, not disposition/format. (databricks-sql-connector's default Thrift-based mode is faster still -- a different, legacy protocol entirely, out of scope here; see that entry's own closing note.) - Every type/value-string mapping was verified against the real workspace before being coded, not assumed from documentation or
databricks-sql-connector's own model classes: manifesttype_nameisBYTE/SHORT/INT/INTEGER/LONGfor the integer widths (notTINYINT/SMALLINT/BIGINT),DECIMALcarries separatetype_precision/type_scalefields with the value string pre-formatted to the exact scale ("3.1400"forDECIMAL(10,4)); every non-null JSON_ARRAY value is a string, including non-finiteDOUBLEs (literally"NaN"/"Infinity"/"-Infinity", which Rust'sf64::from_stralready parses natively -- unlike thearrow-json/ARROW_STREAMpath'snon_finite_floatsworkaround above, no special-casing needed here);TIMESTAMPis always UTC/Z-suffixed RFC3339,TIMESTAMP_NTZis naive with noZ;BINARYis base64. - Falling back is safe to double-execute only for a statement that reached a terminal FAILED state before the second statement is ever submitted -- NOT for one that reached SUCCEEDED, corrected 2026-08-11. This entry originally claimed "FAILED/SUCCEEDED" were both safe, reasoning that either way the first attempt's outcome was "fully known" before resubmitting -- wrong, and a real data-safety bug shipped from it, caught in independent code review, not by any test: FAILED means nothing committed server-side (the statement was rejected before/without really executing, e.g. the INLINE byte-limit-exceeded case below), so a fresh submission is a genuinely distinct, harmless execution -- not the same risk class as blindly retrying an ambiguous mid-flight POST failure, which is what
idempotent(two entries below) guards against instead. SUCCEEDED means the opposite: real rows came back, so for non-idempotent SQL (INSERT/MERGE/UPDATE/DELETE) any write already committed -- resubmitting the identical SQL a second time duplicates it. Two call sites had exactly this bug:pipeline.rs'sexecute_lazy_prefer_inline(JSON-conversion failure on an INLINE result that already reached SUCCEEDED) andclient.rs'sexecute_arrow_statement_prefer_inlineitself (the defensive "SUCCEEDED but nodata_array" arm). Both now return a clearArrowbricksErrornaming the statement instead of resubmitting -- there is no safe alternative that re-fetches the same statement's data a different way either (an INLINE submission has noexternal_links/manifest chunks to fall back to; returning data inline in the response is the entire point of INLINE, so once that data can't be used there's nothing left to resolve for thatstatement_id). Seetests/wiremock_pipeline.rs'sprefer_inline_on_unsupported_column_type_errors_instead_of_resubmitting/prefer_inline_on_missing_data_array_errors_instead_of_resubmitting(both mount only the INLINE-tagged mock with.expect(1)-- a regression back to resubmitting would fail on the missing second mock, not just on the wrongResultvariant) and CHANGELOG.md's own entry.- A second, independent bug was hiding behind the first one, in this session's own test suite:
prefer_inline_falls_back_to_external_links_on_byte_limit_exceeded(the genuinely safe FAILED-state fallback, unaffected by the fix above) mountedinstall_mock_warehouse's generic, unconditionalPOST /statementsmock before the INLINE-tagged byte-limit mock. Per wiremock 0.6.5's ownMountedMockSet::handle_request(a stablesort_by_keyon priority, so a tie between two equally-matching mocks goes to whichever was mounted first -- confirmed directly from its source, matching this file's ownHasDispositiondoc comment), the generic mock actually won the very first (INLINE) request too, since it matches unconditionally and was mounted first -- the byte-limit response was never really served. The test still passed regardless, for the wrong reason: the old, buggyNone => resubmitfallback (see above) silently caught the resulting "SUCCEEDED with no data_array" case and resubmitted anyway, landing on the same generic mock a second time and coincidentally producing the same correct-looking result through a completely untested code path. Removing that unsafe fallback surfaced this immediately as a hardunwrap()panic instead of a silent false-positive pass -- fixed by mounting the INLINE-tagged mock first. Lesson for next time: a test that passes for reasons other than the ones its own name/docstring claims is worse than a missing test -- it actively hides the real gap. If a "safe" fallback test's mock setup relies on tie-breaking between two overlapping matchers, order them deliberately and comment on why (asHasDisposition's own doc comment already tries to for its siblings), not just append a new mock and assume it'll be reached.
- A second, independent bug was hiding behind the first one, in this session's own test suite:
client.rsstays Arrow-free on purpose, even for this feature.execute_arrow_statement_prefer_inlinereturns rawVec<Vec<Option<String>>>rows +ColumnDescriptions (anInlineOrExternal::Inlinevariant) rather than an ArrowRecordBatch-- the actualjson_convert::json_array_to_record_batchcall, and its own error-reporting branch, live inpipeline.rs'sexecute_lazy_prefer_inline, matching this crate's existing "chunk bytes are handed off raw, decoding happens in pipeline.rs" boundary (seeclient.rs's own module doc comment).- See
rust/arrowbricks_core/tests/wiremock_pipeline.rs'sprefer_inline_uses_the_embedded_data_array_with_zero_further_requests/prefer_inline_falls_back_to_external_links_on_byte_limit_exceeded/prefer_inline_on_unsupported_column_type_errors_instead_of_resubmittingandtests/test_cursor.py'stest_prefer_inline_uses_embedded_data_array_with_no_further_requests/test_prefer_inline_falls_back_when_result_is_too_big_for_inlinefor the covering tests, andjson_convert.rs's own#[cfg(test)]module for the per-type conversion unit tests.
- INLINE's byte cap is exactly 26,214,400 bytes (25 MiB) and failure is a clean, typed error, never silent truncation -- confirmed by running the real 5.6M-row/120-column
-
Every statement submission (both
execute_statementandexecute_arrow_statement_prefer_inline, via their sharedsubmit_and_poll) tries to reuse a pooled SEA session (POST /api/2.0/sql/sessions) instead of submitting stateless, on by default -- not a knob. Found and closed as the real cause of a residual ~15-25% gap againstdatabricks-sql-connector's own SEA mode (use_sea=True) afterprefer_inlineitself turned out not to help (see that entry's closing note): a raw side-by-side comparison against a real workspace showed a pooledsession_idcutting mean submit-to-terminal-state latency from 495ms to 404ms (15 warm runs) for an identical small query --databricks-sql-connector's SEA mode creates exactly this kind of session atconnect()and reuses it, arrowbricks previously never did. Re-verified end to end after implementing: session-pooled steady-state mean 433ms/median 410ms against the same real table, now in the same band asdatabricks-sql-connector's own SEA mode (422-461ms) instead of ~20% behind it.- Two hard constraints, both confirmed by direct testing against a real workspace, rule out one shared session per client: (1) Databricks rejects
session_idcombined with per-statementcatalog/schemaoutright (HTTP 400:"The session_id field cannot be set at the same time as the catalog or schema fields") -- a session is created for a specific (catalog, schema) pair, soclient::Pool<T>(client.rs, generic over the pooled item --Pool<String>for SEA's session id,Pool<thrift::SessionHandle>for Thrift's, sharing the same checkout/checkin logic) is keyed on(Option<String>, Option<String>), not a single cached id. (2) Two statements submitted concurrently on the samesession_idcan make the server fail with an internal error -- reproduced directly by firing 3 concurrent statements on one session:"Cannot invoke SparkSession.sessionState() because sparkSession is null". A session is safe for sequential reuse, not concurrent sharing -- this is a real checkout/checkin pool (checkout_session/checkin_session), sized per key (MAX_SESSIONS_PER_KEY = 8, a fixed cap, not exposed as a constructor kwarg since nothing's asked to tune it yet), not a single id handed to every caller. - Every failure mode of this feature falls back to exactly today's pre-session behavior instead of surfacing a new error or blocking. Pool exhaustion (every session for a key already checked out) and session-creation failure both fall back to a plain session-less submission for that one call (catalog/schema set directly on the statement, as before this feature existed) -- verified via a dedicated wiremock test that fires
MAX_SESSIONS_PER_KEY + 1genuinely concurrent statements on one key and confirms exactly one falls back. Any statement that errors while holding a pooled session has that session discarded rather than returned to the pool (a dedicated test confirms a FAILED statement's session is never handed to the next caller on the same key) -- conservative (a plain query error, e.g. bad SQL, still throws away a perfectly good session), but guarantees a session that might be in the same bad state behind the SparkSession-null crash above is never reused. Nothing here ever waits/blocks for a session to free up. DatabricksClient.aclose()/Connection.close()are no longer no-ops -- they close every currently-idle pooled session (DbClient::close_all_sessions, exposed to Python asClient.close_sessions), best-effort (a failed close is just left for Databricks' own server-side session TTL to reap). A session still checked out (a genuinely in-flight statement) at the timeaclose()runs isn't in the idle pool and so isn't closed here -- calling it before every pending statement has finished is a caller ordering issue, not something this method can fix from inside; this is the same class of accepted trade-off as this crate's other "best-effort, TTL reaps the rest" cleanup paths.- Verified end to end against a real workspace, not just wiremock, beyond the raw session-vs-no-session timing above: a
catalog=override still resolves correctly with a session in play (current_catalog()came back correct); 12 concurrentexecute()calls on oneConnection(the exact shape that broke a naive shared-session design) all returned correct, distinct results with no crash, twice in a row (second burst reusing the now-idle pooled sessions from the first);Connection.close()completed cleanly afterward. - See
rust/arrowbricks_core/tests/wiremock_pipeline.rs'ssession_is_created_once_and_reused_across_sequential_statements/session_creation_failure_falls_back_to_catalog_on_the_statement_body/concurrent_statements_on_the_same_key_each_get_their_own_pooled_session/session_pool_exhaustion_falls_back_to_session_less_submission/a_failed_statement_discards_its_session_instead_of_returning_it_to_the_pool/close_all_sessions_deletes_every_idle_pooled_sessionfor the covering tests.
- Two hard constraints, both confirmed by direct testing against a real workspace, rule out one shared session per client: (1) Databricks rejects
-
ApiError::from_reqwesttakes anidempotent: bool--transientcan only betruewhen replaying the whole request is actually safe. Found against a real workspace: a 400-chunk/5.6M-row full-table fetch failed outright with reqwest's "error decoding response body" (Kind::Decode) on one of many large concurrent blob downloads -- a connection that closed mid-body, not a permanently broken request. Reqwest wraps any body-read failure (not just content-decoding) in this sameKind::Decode(seeasync_impl::response::Response::do_bytes);is_request()(excludingis_connect()/is_timeout()) additionally covers hyper's pooled-connection-reuse race ("connection closed before message completed"), which fires before any body is read -- reasoned from reqwest/hyper's own source, not independently reproduced: a directed attempt to force it (a standalone client re-using onereqwest::Client's connection pool against a server that responds once then immediately closes, zero delay before the second request) self-healed 20/20 times -- hyper's own idle-connection health check evidently detects the closed peer and transparently opens a fresh connection before handing the dead one out for reuse, at least under that simple, low-concurrency scenario. Kept anyway because it's safe regardless (scoped to idempotent requests only, same asis_decode()) and the TOCTOU window this covers may still be reachable under real production load (64-way concurrency, real network jitter) even though a quick local test couldn't force it -- but treat this specific classification as a plausible defensive measure, not a confirmed fix for an observed failure the wayis_decode()is (that one was hit for real on the 400-chunk fetch and reproduced with a directed test). Before this fix, every reqwest-level error was hardcoded non-transient, so a single mid-transfer blip permanently failed the entire query with zero retry -- reproduced identically on the pre-fix build (ruled out the pre-resolved-link change above as the cause before looking further).idempotentmust befalsefor the statement-submit POST -- a decode/mid-flight-send failure there means the server may have already accepted and started executing the statement before the response broke; for arbitrary caller SQL (INSERT/MERGE/COPY INTO), blindly replaying that POST risks a second, duplicate execution, not just a duplicate read (an earlier draft of this fix retried unconditionally on any reqwest error regardless of method, caught in code review before it shipped).authed_jsonpassesidempotent = method == GET;fetch_link_bytes,upload_volume_file(PUT withoverwrite=true), anddelete_volume_file(DELETE, 404 already treated as success) passtrueunconditionally -- all three are safe to blindly replay by construction. Connect/timeout errors are deliberately still non-transient, unchanged -- those mean the endpoint isn't responding at all, where failing fast on the caller's own timeout is still correct. Seeclient.rs'sfetch_link_bytes_retries_after_a_connection_closed_mid_body(a rawTcpListenerthat claims moreContent-Lengththan it sends then closes, since wiremock has no way to violate its own Content-Length) for the regression test. -
stream_query_json'snon_finite_floats="string"recovers NaN/Infinity from the JSON writer's own fixednullcollapse -- opt-in, default stays"null".arrow-json(a third-party crate, not something arrowbricks wrote) hardcodes non-finite floats tonull-- valid JSON, but indistinguishable from a real SQL NULL once it's out (found by testing edge-case data types against a live warehouse: a NaN and a NULL in the same column both came back asnull).encode_ndjson_lines'snon_finite_as_stringpatches those specific cells to"NaN"/"Infinity"/"-Infinity"afterward viareplace_nth_top_level_null, which finds the target by schema field position, not by key name matching the JSON text -- correct even when two top-level columns share the same name (a real, tested case) -- and only ever replaces a value already known, from the source Arrow array (not by inspecting the JSON), to be exactly the 4-bytenulla non-finite float produces, so it never touches a real NULL. Only top-level Float32/Float64 columns are covered; one nested inside a STRUCT/ARRAY/MAP still comes back asnulleither way. -
A timed-out (
total_timeout_s) background task is genuinely joined before the error reaches the caller, not just.abort()-requested.HeartbeatWait/HeartbeatStream::tick()both run the wrapped operation onpyo3_async_runtimes's own persistent background runtime (needed so this works from a synchronous PyO3 method), independent of whatever's callingtick().JoinHandle::abort()only requests cancellation -- the task keeps running until its next await point. Found by testing a real timeout against a live warehouse from a short-lived script: without awaiting the aborted handle, a caller could seeQueryTimeout, decide the program was done, and exit while the orphaned task was still mid-drop; if that drop needed to touch a Python object (e.g. atoken_providercallback reference) after the interpreter had started finalizing, it panicked with "the Python interpreter is not initialized". Fixed by awaiting the aborted handle before returning the timeout error in bothtick()s -- fully closes the race for this path, verified with 3/3 clean repro runs after the fix (0/3 clean before it). If you touch either, keepheartbeat.rs'stotal_timeout_waits_for_the_aborted_task_to_actually_drop/heartbeat_stream_timeout_waits_for_the_aborted_task_to_actually_drop(a task that sets a shared flag in its ownDropimpl, assertedtrueimmediately aftertick()returns, not merely "eventually").- This does NOT fully cover Python-side cancellation (
asyncio.wait_for,task.cancel()) of the surrounding coroutine -- a known, accepted residual gap, not an oversight. That dropsHeartbeatWait/HeartbeatStreamdirectly, never going throughtick()'s timeout branch at all -- andJoinHandle::dropalone does not abort a task (tokio leaves it running fully detached), so without aDropimpl nothing would ever even request cancellation.Drop for HeartbeatWait/Drop for HeartbeatStreamcall.abort()on any still-held handle to close that gap, butDrop::dropcan't.await, so this can only request cancellation, not confirm it finished, unlike thetick()path above. Reproduced against a live warehouse: intermittent panic (matching this same "Python interpreter is not initialized" message) in ~3/10 runs both before and after theDropfix -- but in every single run, panicked or not, the calling Python program's own exit code was0and it completed normally regardless; this is a background thread's cosmetic stderr noise from an already-irrelevant orphaned task, not a correctness or control-flow issue for the caller. Fully eliminating it would mean either patching howpyo3_async_runtimes::tokio::future_into_py's own Python-side cancellation propagates into the Rust future it wraps (apyo3-async-runtimeslimitation, not something this crate's code controls), or synchronously blocking insideDropto join the task (a real anti-pattern inside an async runtime) -- neither judged worth it for a cosmetic-only residual case that a long-running server (the actual target use case) never hits at all, since its interpreter never finalizes mid-query. Seeheartbeat.rs'sdropping_heartbeat_wait_directly_requests_cancellationfor what theDropfix does prove (cancellation is requested, eventually observable viaAbortHandle::is_finished()) versus what it deliberately does not claim.
- This does NOT fully cover Python-side cancellation (
-
ResultStream(the lazyexecute()+fetchmany_arrow/fetchall_arrowpath -- whatCursor.fetchall/fetchmany/fetchall_arrowactually use) poisons itself if afetch_at_leastcall ever exits early, including by being cancelled. Found in code review and reproduced:fetch_at_leastpulls chunks off itsReorderBufferinto a localdecode_handleslist, decoding and only appending toself.pending/self.pending_rowsafterward. Dropping the whole future mid-flight -- exactly what a Pythontask.cancel()/asyncio.wait_fortimeout does viapyo3_async_runtimes, which does propagate that cancellation into the wrapped Rust future -- abandons whatever was indecode_handlesat that moment; those chunks are already consumed out of the reorder buffer with nothing left holding their results, but the buffer's own bookkeeping (andself.exhausted) has already moved past them. Before the fix, a caller who caught the timeout/cancellation and retriedfetchall()on the sameCursorgot a real but silently truncated row count (reproduced: 20 of 30 expected rows) with no error at all.PoisonOnDrop(aDrop-guard defused only right beforefetch_at_least's own finalOk(())) sets apoisonedflag on any early exit -- a later call checks it up front and errors immediately instead of proceeding as if nothing happened. This also covers a genuine (non-cancellation)Errfrom a chunk fetch mid-batch, which loses not-yet-decoded handles from that same batch the same way -- a caller retrying after seeing any error from this path gets a clear "re-run the query" error, not a silent partial success. Seepipeline.rs'sPoisonOnDropdoc comment andtests/wiremock_pipeline.rs'slazy_fetchall_errors_instead_of_silently_truncating_after_a_cancelled_fetch(races a realtokio::time::timeoutagainst a real delayed mock response -- not a simulated flag) plustests/test_cursor.py's Python-level companion.run_pipeline/ExecuteResult(the eager, one-shot path) doesn't need this -- nothing persists it across separate calls the wayCursorholds and reuses aResultStream. -
PyTokenProviderre-captures its cachedTaskLocalson every call from a context that has one, not just the first ever. Needed because worker tasks spawned byfetch_chunks_with_backpressuredon't inherit the outer task's asyncio-event-loop context (see the entry above this one inlib.rs's own doc comment for that original fix) -- but caching the first-ever captured locals forever pinned the wholeDbClient(which persists across many separateexecute()calls, by design) to whichever event loop happened to be running the very first time an asynctoken_providerwas called. Found in code review and reproduced: a secondasyncio.run()using the same client and an asynctoken_providerfailed with "Event loop is closed" instead of just using the current loop. Fix removes theif guard.is_none()condition, so every call from a context with its own loop (the outer task, always --execute_arrow_statementneeds a token before any worker is spawned) refreshes the cache; a worker task's own capture attempt still fails (no loop of its own) and falls through to whatever's cached, which is safe since the outer call for that statement already refreshed it moments earlier. Only affects asynctoken_providers -- a sync one never readslocalsat all. Seetests_py/test_token_provider.py'stest_async_token_provider_survives_a_new_event_loop_across_separate_asyncio_runs(two genuinely separateasyncio.run()calls, not one test-managed loop). -
Cursor.execute()/execute_streamed()clear_result/_schema/_manifest_descriptionup front, not just on success. Found in code review: a secondexecute()call that itself failed (statement FAILED/CANCELED,QueryTimeout) used to leave all three still pointing at the previous successful statement, with no error of its own -- a caller doingtry: await cur.execute(sql) / except: ...and reading the cursor anyway got the wrong query's rows and description silently. Seetests/test_cursor.py'stest_failed_second_execute_clears_the_previous_result_set. -
ApiError::from_status'stransientclassification takesidempotenttoo, same asfrom_reqwest. Found in code review: it didn't, so a 429/5xx on the statement-submit POST was retried regardless of method, bypassing the whole reasoningfrom_reqwest'sidempotentparam exists for. 401/403/408/429 stay unconditionally transient either way (they mean the request was rejected before any processing started -- auth failure, rate limit, client timeout -- safe to retry regardless of method); only the murkier 5xx case (which can mean the backend already accepted and started the statement before a later failure, e.g. a gateway timeout, produced the 5xx anyway) is now gated onidempotent. -
decompress_lz4_frameloops while the underlying reader still has bytes left (decoder.get_ref().is_empty()), not while output keeps growing. Found in code review: a real, valid LZ4 Frame that happens to decode to zero bytes (header immediately followed by an EndMark -- a legal shape, not malformed input) makes oneread_to_endcall returnOk(0)without erroring and without necessarily finishing that frame's own bytes in the same call -- the oldout.len() == beforecheck read that as "no more frames" and stopped, silently dropping every frame concatenated after the empty one. Same silent-truncation shape as the original multi-frame bug this loop exists to fix. Seeclient.rs'sdecompress_lz4_frame_survives_a_zero_content_frame_in_the_middle. -
Client/DatabricksClientreject passing bothtokenandtoken_provider, not just neither. The doc comments always said "exactly one of the two," but the validation only ever checked "at least one" -- found in code review. Both the RustPyDbClient::newand PythonDatabricksClient.__init__now raise/error on(Some, Some)too. -
Three known, accepted gaps from the same code review pass, not fixed here -- deliberately deferred, not overlooked:
Cursor.description'stype_namevocabulary silently switches from Databricks SQL manifest names ("LONG","STRING") to arrow-rsDataType::to_string()output ("Int64","Utf8") once the real schema is known after the first fetch -- the preference for the real schema over the manifest estimate is deliberate and documented (seecursor.py's own comment), but the vocabulary change itself isn't, and README.md documentsdescriptionas a flat, presumably-stable(name, type_name, ...)contract. No test anywhere currently asserts ondescription[i][1]'s actual value. Whether to normalize the vocabulary (and to which one) or just document the switch explicitly is a judgment call for whoever hits it in practice, not obviously one answer.ReorderBuffer::next()(pipeline.rs) drains the bounded backpressure channel into an unboundedpending: HashMapwhenever a lowchunk_indexis slow to arrive relative to later ones --client.rs's own comment onfetch_chunks_with_backpressureclaims peak buffered chunks stays "at ~concurrency, not O(whole result)," which isn't quite true if one early chunk is pathologically slow: workers keep fetching later chunks into the buffer since the channel itself keeps draining, even thoughpending_rows/consumption is blocked waiting on the slow one. Needs a genuinely slow single chunk among many fast ones to bite; on the 400-chunk/5.6M-row table that's a real amount of memory if it happens, but it's a rare shape and fixing it properly (boundingpending's own size, not just the channel's) is a bigger design change than a quick patch._streaming.py'sawait_with_heartbeathas a narrow race betweenasyncio.wait's timeout returning and theloop.time() >= deadlinecheck right after: a task that completes in that tiny window has its result awaited and silently discarded, thenQueryTimeoutis raised instead of the real result. Also,contextlib.suppress(asyncio.CancelledError)around the finalawait taskonly swallows cancellation -- if the task had instead failed with a different exception in that same window, that exception propagates in place ofQueryTimeout. Genuinely narrow (needs the task to finish in a sub-millisecond gap), and combined with theResultStreampoisoning fix above, no longer silent (a caller who then retries on a poisoned stream gets a clear error either way) -- judged not worth the added complexity of closing a race this narrow on its own.
-
protocol="thrift"(the default onClient/DatabricksClient-- see the "Thrift is now the default" entry below for when/why this flipped;protocol="sea"remains a fully-supported, explicit alternative) speaks the same HiveServer2-compatibleTCLIServiceprotocoldatabricks-sql-connectoruses by default (when its ownuse_seaisn't set) -- plain HTTPS POST,TBinaryProtocol-encoded, no framing beyond HTTP itself. Built to close the gapprefer_inlineand SEA session-pooling didn't (see those entries' own closing notes) -- confirmed against a real workspace:TExecuteStatementReq.getDirectResultscan return a small result's data inline in the same RPC that submits the statement, where SEA always needs at least a separate poll/fetch round trip. Every struct's field IDs/types (thrift.rs) were read directly out ofdatabricks-sql-connector's own installed, Thrift-compiler-generatedttypes.py-- the real wire-format spec, not guessed at or taken from public Hive/Thrift docs, which don't cover Databricks' own extension fields (IDs >= 1281). Only the 7 RPCs this crate actually needs are implemented (OpenSession/ExecuteStatement/GetOperationStatus/FetchResults/CloseOperation/CloseSession/CancelOperation) -- no catalog/table/column-browsing surface, since this crate has none. A hand-rolled reader/writer was chosen over thethriftcrate: the actual wire format needed is a few hundred lines of straight-line code, cheaper than a whole generic RPC framework for 7 RPCs' worth of concrete structs (see "Dependencies" above).OperationStatusResp::read's "display message" field id was wrong (12, not the real 1281) until fixed in the same session this file's Thrift mock test infra (below) was built. Found by cross-checkingdatabricks-sql-connector's own real, installedttypes.pywhile building mock response bytes against its real field layout --TGetOperationStatusResp.thrift_spechas no field 12 at all;displayMessageis field 1281. Before the fix, a real server'sGetOperationStatus.displayMessagewas silently skipped (unrecognized field id, generically skipped, not an error) -- low real-world impact sinceterminal_error()checks the top-levelTStatus.displayMessage(field 6, correctly mapped, a distinct field on a distinct struct) before ever falling back to this one, so a real error's message still came through correctly in practice; this was a real but narrow bug, never independently observed to matter on a live query. Seethrift.rs's ownoperation_status_resp_reads_display_message_from_field_1281_not_12regression test.- Large results still cloud-fetch to blob storage even over Thrift (
TRowSet.resultLinks, alongsidearrowBatchesfor small/inline results) --pipeline.rs'sdrive_thrift_fetch_loopreuses the existingfetch_link_bytes/decompress_lz4_frame/decode_chunkmachinery for that path unchanged; only the submit/session/poll/fetch-metadata layer is genuinely new. - A cloud-fetch file can contain more rows than its own link's declared
rowCount, and must be truncated to it. Confirmed against a real workspace, not hypothetical:SELECT * FROM benchmark_table LIMIT 500000came back with 502,879 rows end to end via this path before the fix (2,879 extra) --databricks-sql-connector's ownResultSetDownloadHandler.runhas an identical check with an identical justification in its own comment ("The server rarely prepares the exact number of rows requested... we drop the extraneous rows in the last file").pipeline.rs'struncate_to_declared_row_countdecodes, slices to the declared count (only ever drops rows, never guesses which to keep beyond "the first N, in order"), and re-encodes -- a no-op when the file already matches, which is the common case. Seetruncate_to_declared_row_count_slices_the_straddling_batch/_is_a_no_op_when_not_needed/_skips_truncation_when_bound_is_unknown.- The identical over-delivery happens on the inline
arrowBatchespath too, and this same fix was never applied there -- found by a real-warehouse variety pass, not inspection.run_thrift_fetch_loop'sarrow_batchesbranch always passedtruncate_to: None, trusting the decoded blob's row count outright, even thoughbuild_inline_blobalready sums eachArrowBatch's own declaredrow_countfield (the exact same per-item declared boundResultLink.row_countprovides on the other path) and had it sitting right there, unused for this purpose. Caught by a deliberately varied battery of real queries againstbenchmark_table(boundary sizes around the split threshold,WHEREfilters,GROUP BY,DISTINCT, struct columns, narrow projections) run after the Range-split change, specifically to catch anything that change might have disturbed -- it hadn't touched this path at all, but the battery caught a pre-existing bug anyway:SELECT id, name FROM benchmark_table WHERE is_one_off = true LIMIT 5000came back with 5037 rows via this crate's Thrift path, whiledatabricks-sql-connectorreturned exactly 5000 on both of its own protocols for the identical SQL -- ruling out a server-side Thrift/HiveServer2 quirk and confirming a real client bug. Fixed by passingtruncate_to: Some(row_count)instead ofNone, reusingdecode_chunk_item's existing truncation, not a new mechanism. Seethrift_inline_arrow_batch_is_truncated_to_its_declared_row_count.
- The identical over-delivery happens on the inline
- A session checked out for one statement (pooled or a throwaway one,
client::Pool<thrift::SessionHandle>) must stay open until that statement's operation is fully drained and closed -- not just until it reaches a terminal "finished" state. Found by testing genuine concurrent load (12-20 concurrent statements on oneConnection) against the real warehouse, not by any unit or mock test: the first version of this feature closed a throwaway session (opened whenMAX_SESSIONS_PER_KEYwas already exhausted) immediately afterExecuteStatementreturned, before its own background fetch loop had drained the result -- intermittently (~25-30% of runs once concurrency exceeded the pool size, reproduced across 20+ repeated trials) crashing that same still-in-flight fetch withRESOURCE_DOES_NOT_EXIST: Command ... does not exist. Closing a Thrift session invalidates every operation still open under it, including its own -- the close was simply too early, not a fundamentally unsafe idea. Fixed by deferring a throwaway session's close (and the operation's ownCloseOperation) todrive_thrift_fetch_loop's single cleanup point (run_thrift_fetch_loopfactored out specifically so a bare earlyreturninside the fetch loop can never bypass it), which always runs once the loop is fully done, on every exit path -- not right after submission. Re-verified clean across 18 further concurrent runs (N=12 and N=20, both well pastMAX_SESSIONS_PER_KEY) after the fix, 0 failures. A pooled session is still checked in eagerly, right afterExecuteStatementreturns, unlike the throwaway case -- that only returns it to the idle pool for potential reuse by a different operation, doesn't close it, and a HiveServer2-compatible session is designed to support multiple independently-addressed operations at once; the crash was specifically about closing a session out from under one of its own still-open operations, not about a session merely being idle or shared. This is the Thrift-side analogue of the SEA session pool's own "discard on any error, never risk reusing a possibly-corrupted session" caution above, applied to a different failure mode. - Concurrency safety here has NOT been fully independently stress-tested for the pooled (not throwaway) session-reuse path the way SEA's was -- i.e., whether a session checked back into the idle pool and picked up by a second, concurrent
ExecuteStatementwhile its first operation is still being fetched is safe, versus merely "not yet observed to fail." Reasoned as safe from HiveServer2's own multi-operation-per-session design (see above) and consistent with every real-workspace test run so far (including the concurrency fix's own 18-run re-verification, which does exercise session reuse under load), but this is inference from protocol semantics, not a targeted, deliberately-constructed reproduction the way the throwaway-close bug above was. Worth a dedicated test if this path sees heavy production concurrency. prefer_inlineis a silent no-op underprotocol="thrift", not an error -- Thrift has no INLINE-disposition equivalent, andgetDirectResultsalready covers the small-query case it exists for on SEA.run_thrift_fetch_looppipelinesFetchResultsdiscovery with a bounded worker pool downloading across every discovered batch concurrently, not just the current one -- this was NOT true of the first version of this backend, and mattered for real. Confirmed against a real workspace (benchmark_table,LIMIT 500000, 4 warm runs each): the original loop (fully await one batch's downloads before ever asking for the next batch's links) averaged 20.5s vs SEA's 11.8s, ~1.7x slower, consistent across every run. Root cause: every SEA path (execute_lazy/execute_lazy_prefer_inline/run_pipeline/run_json_pipeline) callsclient.rs'sfetch_chunks_with_backpressure-- the entire chunk manifest is known upfront, so up tochunk_fetch_concurrency(64) downloads run concurrently across the whole result immediately -- while Thrift'sFetchResultsRPC only reveals one batch ofresultLinksper call, and the original loop serialized "ask for the next batch" behind "finish downloading the current one," capping effective concurrency at whatever oneFetchResultsresponse happened to contain. Fixed by splitting into a producer (the sequentialFetchResultsloop -- Thrift's own cursor semantics require this side to stay strictly sequential, concurrentFetchResultscalls on one operation aren't a thing this protocol supports) that pushes each discovered link into a boundedmpscchannel instead of downloading it directly, and a fixed pool ofchunk_fetch_concurrencyworkers sharing that channel's receiving end (Arc<tokio::sync::Mutex<Receiver>>, the standard way to turn onempsc::Receiverinto an effective multi-consumer queue) that download concurrently across however many batches have been discovered so far --Sender::send's natural backpressure once the buffer fills lets the producer keep racing ahead on cheap metadata-only round trips while downloads catch up, same "peak buffered stays at ~concurrency" trade-offfetch_chunks_with_backpressure's own doc comment describes.chunk_indexis still assigned once, deterministically, at discovery time in the producer -- downloads completing out of order (now genuinely possible across batches, not just within one) is exactly whatReorderBufferalready exists to handle. Re-verified clean after the fix: 500k rows, 4 warm runs, Thrift mean 11.6s vs SEA's 11.9s (on par, not just "less bad"); 2M rows, Thrift's one warm run 33.3s vs SEA's 34.6s -- holds at larger scale too, not a small-result fluke.- Verified end to end against a real workspace beyond the above: STRUCT columns,
ARRAY/MAP, NaN/Infinity/NULL (native Arrow types viauseArrowNativeTypes, no JSON-string-collapse issue the SEA/prefer_inlinepath has), named parameters, catalog/schema session-namespace binding (current_catalog()round-tripped correctly under concurrent load), and clean session-pool close onConnection.close()/aclose().
-
Thrift is now the default protocol (flipped from SEA in this same session, 2026-08-06), with SEA remaining a fully-supported, explicit opt-in (
protocol="sea"). The Thrift entry above documents the wire format and its own real-workspace verification; this entry is specifically about the decision to make it the default. Why: across this session, Thrift was benchmarked directly against SEA on a real production warehouse (see the closing notes onprefer_inlineand the SEA session-pool entries above, plus the Thrift entry's ownrun_thrift_fetch_looppipelining fix) and found to be never slower than SEA on any query shape tested, and roughly 2x faster for small queries --TExecuteStatementReq.getDirectResultsreturns a small result inline in the same RPC that submits the statement, where SEA always needs at least one further poll/fetch round trip; for large, multi-chunk results the two land in the same band once Thrift's own fetch loop pipelinesFetchResultsdiscovery against its download worker pool (the fix documented above). Given that, defaulting new callers onto the strictly-faster-or-equal path was judged a clear win, not a close call.- The blocker to flipping the default was test coverage, not the backend itself: essentially every existing test in this repo constructed
DatabricksClient/Clientwithoutprotocol=at all (SEA was always the implicit default), against mock HTTP servers that only understood SEA's REST/JSON shape -- flipping the default naively would have silently broken dozens of tests from a coverage gap, not a real regression. Real Thrift-speaking mock infrastructure was built first, in both languages, specifically so the new default gets the same depth of test coverage the SEA path already had, rather than the flip riding on a thinner safety net.- Rust (
rust/arrowbricks_core/tests/wiremock_thrift.rs): wiremock matches on HTTP method/path, but every Thrift RPC hits the one shared path (/sql/1.0/warehouses/{id}) --IsThriftRpc, a customwiremock::Match(mirroring this same directory'swiremock_pipeline.rs's ownHasDispositionmatcher, which solves the identical "route by body content, not path" problem for SEA'sdispositionfield), parses the Thrift message name out of the request body viathrift::Reader::read_message_beginand routes on that. Response bytes for every RPC are built directly withthrift::Writer-- the exact same primitivesthrift.rsitself uses to build requests, since the wire format is symmetric; no new Cargo dependency, consistent withthrift.rs's own "hand-rolled beats a whole RPC framework for 7 RPCs" reasoning. Covers: a happy-path small query returning data inline viagetDirectResults(.expect(0)proves zeroFetchResults/GetOperationStatuscalls, not just a correct result some other way); a multi-batch query with several sequentialFetchResultscalls,resultLinksin each, downloaded concurrently with genuinely out-of-order completion forced via reversed per-chunk delays, order preserved end to end; aFAILEDstatus surfacing both viagetDirectResults' own immediateoperationStatusand via polledGetOperationStatus; LZ4-compressedresultLinksblobs and LZ4-compressed inlinearrowBatches, both confirminglz4_compressedmetadata is actually honored (not just requested); and the Thrift session pool (client::Pool<thrift::SessionHandle>/thrift_checkout_session/thrift_checkin_session), which had zero mock coverage before this (only ever verified against the real warehouse) -- session reuse across sequential statements, discard-on-FAILED-statement, and pool-exhaustion falling back to a throwaway session that still succeeds and gets closed exactly once (mirroring the SEA session pool tests' shape, adapted for Thrift's one real difference: no session-less fallback exists, so exhaustion falls back to a throwaway session instead of a session-less statement body). - Python (
tests/thrift_mock.py, duplicated self-contained inrust/arrowbricks_core/tests_py/thrift_mock.py-- this directory has no shared conftest today, each test file rolls its own inline mock server, so this follows that existing convention rather than introducing cross-directory sharing): built ondatabricks-sql-connector's own installed, real, Apache-Thrift-compiler-generateddatabricks.sql.thrift_api.TCLIServicemodule (ttypes.py's structs,TCLIService.py's realProcessordispatch) rather than hand-rolling a second Python Thrift codec -- a deliberate, considered choice, not the path first assumed:ttypes.pyis not just "a" Thrift library, it's the exact filethrift.rs's own field IDs were read from (see the entry above), so building mock responses against it directly is strictly more trustworthy than a second hand-written parser that could carry independent bugs, and it paid for itself immediately -- cross-checking against it is what caught theOperationStatusRespfield-1281 bug above. A hand-rolled.thriftIDL loaded viathriftpy2was also considered and rejected: it would reintroduce exactly the "second hand-authored source of field IDs" risk this approach avoids. Added as a test-only entry inpyproject.toml's[dependency-groups] dev(never imported by the shipped package, same "zero required runtime dependencies" reasoning asarro3-core).Processor.processitself reads the RPC name out of the request body and dispatches, so (unlike the Rust mock) no hand-written routing-by-name matcher is needed at all. Covers the same shape as the Rust suite at both theCursor/DatabricksClientlevel (tests/test_thrift_pipeline.py) and the lower-levelarrowbricks_core.Clientlevel (rust/arrowbricks_core/tests_py/test_thrift.py, backed by a new, minimalrust/arrowbricks_core/tests_py/conftest.pyproviding just themock_thrift_serverfixture -- this directory's first conftest.py, added because this one fixture is genuinely shared infrastructure, not per-test inline setup): happy path, multi-chunk order preservation, FAILED-statement error propagation (both immediate and polled), LZ4-compressedresultLinks, session reuse across sequential executes, and (at the lowerarrowbricks_core.Clientlevel, where the mock'sHandler.ExecuteStatementreceives the real, fully-parsedTExecuteStatementReqfor free) thatparametersactually reachesTSparkParameteron the wire.
- Rust (
- Every existing test that constructed a client without
protocol=hadprotocol="sea"added explicitly acrosstests/,rust/arrowbricks_core/tests_py/, andrust/arrowbricks_core/tests/wiremock_pipeline.rs(the latter via.with_protocol(Protocol::Sea)on eachDbClient::new(...)) -- so every pre-existing test keeps exercising exactly the backend it always tested, not a coverage gap papered over by an implicit default. - A follow-up review pass on this same flip found and fixed a real, if narrow, correctness gap in the
compressed_flagfix documented above, plus tidied a leftover inconsistency the flip itself introduced:- A link discovered before compression is authoritatively confirmed could still be queued for download against a stale guess. The
compressed_flagfix (see above) closed the captured-once-at-spawn race, butresultSetMetadataandresults.resultLinksare independent optional fields onTFetchResultsResp-- nothing guaranteed a response carrying links also carried the metadata confirming their real compression, so a response with links but no metadata would still have those links queued (and possibly downloaded) againstclient.compress_results()'s initial guess before a later response ever confirmed the real value. Not confirmed to actually occur against this project's real test workspace (which has always returned metadata promptly), but reachable by the Thrift struct's own optional-field typing, and the fix is cheap:run_thrift_fetch_loopnow buffers a batch's links locally instead of queueing them immediately whenevermetadata_confirmedis stillfalse, flushing the buffer the moment metadata is confirmed (same iteration or a later one) or, failing that, once more at loop end using whateverlz4_compressedholds by then (the request's owncanDecompressLZ4Result, same fallback the flag already started from) -- no link is ever handed to a download worker before compression is known at least once. DbClient::new's own internal default was stillProtocol::Sea, only the PyO3/Python-facing layer defaulted to"thrift"-- a deliberate choice at flip time (Rust-only callers, i.e. this crate's own test suite, always call.with_protocolexplicitly anyway), but flagged in review as a real foot-gun for any future Rust-only caller who constructs aDbClientdirectly and forgets to, silently getting SEA while believing they're on the new default. Since every SEA-testing call site already sets.with_protocol(Protocol::Sea)explicitly (the migration above), makingDbClient::new's own defaultProtocol::Thrifttoo cost nothing and removed the divergence entirely -- one default, not two that happened to agree.- Also: the two
decode_chunk_itemtruncation tests that lost their old byte-identity assertions when that function switched from returning re-encoded bytes to returning batches directly (see the "double-decode" entry above) now check actual cell values again, not just row counts -- the row-count-only version couldn't have caught a value/column-order corruption bug on the no-truncation path. Anddecode_chunk_item's "kept never ends up empty" invariant (guaranteed by the surrounding arithmetic, but no longer defended by the old function's explicit error once it stopped needingkept.first()for a schema) got adebug_assertback, plus a test pinningTHRIFT_DIRECT_RESULTS_MAX_BYTES's exact value so an accidental revert (e.g. during a merge conflict) can't silently reintroduce the round-trip regression that constant's own doc comment describes.
- A link discovered before compression is authoritatively confirmed could still be queued for download against a stale guess. The
- The blocker to flipping the default was test coverage, not the backend itself: essentially every existing test in this repo constructed
-
drive_thrift_fetch_loopdrops itsmpsc::Sender<ChunkItem>(tx) right afterrun_thrift_fetch_loopreturns, before the two cleanup RPCs (CloseOperation, and a throwaway session'sCloseSession), not after this whole function returns. Found because the official connector was measurably faster than arrowbricks-thrift on a medium (10k-row) real-table query even though the two traced identically on every leg that should matter --ExecuteStatementRPC latency tied, cloud-fetch download of the identical wire bytes tied (arrowbricks if anything faster raw throughput), LZ4 decompress and the Arrow-IPC decode itself both negligible (~5ms combined for a 13MB blob, confirmed via a local, network-free benchmark against a captured real blob -- decode was never the bottleneck). The actual gap:ReorderBuffer::next's finalrx.recv().await-- the one that returnsNoneto tellResultStream::fetch_at_leastthe result is fully drained -- can only return once every clone oftxis gone, and the original (non-cloned)txwas previously held alive untildrive_thrift_fetch_loopitself returned, which is on the far side ofCloseOperation's own network round trip. So every Thrift query blocked its caller for that RPC's full duration after every row had already been downloaded, decompressed and decoded -- traced against the real warehouse, the time spent in that lastrecv()matchedCloseOperation's own duration to within 0.1ms on 8/8 runs (116-183ms). Fixed with onedrop(tx)line; safe becauserun_thrift_fetch_loopjoins every download worker (each holding its owntx.clone()) before returning, and neither cleanup call has any path back to the consumer -- they still run to completion, just detached, which is what "best effort" already meant. Verified via an interleaved A/B (16 warm runs, alternating old/new in one process to cancel network drift): median end-to-end query time 1043.7ms -> 853.8ms, now at or under the official connector's own ~780-790ms on this same query shape.thrift_pool_exhaustion_falls_back_to_a_throwaway_session_that_still_succeeds(wiremock_thrift.rs) had to change from asserting the throwaway session's close count immediately after every task'sfetchall_arrow()returns to polling briefly first -- that assumption (stream-drained implies cleanup-done) was only ever true because of the bug this fix removes. SEA has no equivalent issue:fetch_chunks_with_backpressure(client.rs) drops its owntxright after its download workers join, with no cleanup RPC in between -- this class of bug is Thrift-only, from its extraCloseOperation/CloseSessionstep.- Trade-off worth knowing, found in a later review pass, not a bug to fix: before this change, the consumer's channel-close being gated on
drive_thrift_fetch_loop's full completion meantCloseOperation/CloseSessionhad always already finished by the time a caller regained control -- a pure side effect of the bug, never a designed guarantee. Now the caller can proceed (and, for a short-lived script, exit the process) while that cleanup is still an in-flight, detachedtokio::spawntask against a process-lifetime runtime (pyo3-async-runtimes'static OnceLock<Runtime>, no explicit shutdown-and-wait). A script that runs one query and exits immediately now has a real, if narrow, window to cut that cleanup off mid-RPC, more often than before. Not a new failure mode -- both calls were already explicitly "best effort," with a failed close already documented to just leave the session for Databricks' server-side TTL to reap -- just a higher-probability instance of an already-accepted one. Long-lived connections issuing multiple queries are unaffected. - One secondary finding from the same investigation, not yet acted on:
run_thrift_fetch_loop'smetadata_confirmed(see the buffering fix above) is only ever set from a realFetchResultsresponse, never fromdirectResults.result_set_metadataeven thoughsubmit_and_await_thrift_statementalready readslz4_compressedauthoritatively from it -- every link from an initial direct rowset is therefore held back and flushed at loop-end instead of immediately. Costs nothing today (confirmed: a 500k-row/33-link query withhas_more=falsestill flushes 0.4-1.0ms later, zero extra RPCs) but would cost a full extraFetchResultsround trip for a result large enough to sethas_more=trueon its direct rowset (pastTHRIFT_DIRECT_RESULTS_MAX_ROWS/_BYTES). - The other secondary finding -- cloud-fetch TTFB -- was investigated further and resolved; see the
download_slots/split-download entry below for what it actually led to.
- Trade-off worth knowing, found in a later review pass, not a bug to fix: before this change, the consumer's channel-close being gated on
-
A single cloud-fetch link downloads over parallel HTTP Range requests when the shared
download_slotsbudget (client.rs, sized tochunk_fetch_concurrency) has room to spare -- e.g. a single-chunk result, where one TCP stream previously left the link mostly idle. Follow-up to theCloseOperationfix above: even after that fix, the connector was still measurably faster on the same 10k-row query. Chased it down to confirm/deny whether cloud-fetch TTFB (70-99ms warm) was a fixable connection-pooling gap -- it wasn't (reqwest's defaults already reuse the connection to blob storage correctly, confirmed from its own source and measured directly: pooled requests hit 51-114ms TTFB vs 204-278ms cold; the residual ~190ms cold-connect cost is Azure Blob Storage's own TLS handshake, identical for the official connector, ruled out cert verification/TLS version/ALPN/DNS as causes one by one) -- but that dead end surfaced the real lever: a real trace of the query showed one 6.27MB link downloaded by one of the 64 configured workers on one TCP connection, the other 63 sitting idle, and that single stream was ~60% of the wall clock. Splitting one link across parallelRangerequests (Azure SAS URLs honorRange) cut a 10k-row query's download time roughly in half -- but a fixed split factor actively regressed a large multi-chunk query (all workers already busy; splitting further just adds request overhead with no spare capacity to absorb it: measured 8-way fixed split at 10.7s vs 8.0s unsplit on a 300k-row/19-link query). Fixed by making the split budget-aware instead of fixed:download_slots, atokio::sync::Semaphoresized tochunk_fetch_concurrency, gives each link download one mandatory permit plus up toMAX_SPLIT_PARTS - 1(7) more if currently available -- a single in-flight link claims up to 8-way parallelism, while 19 in-flight links exhaust the budget and mostly get one stream each, same as before this change. Verified against the real warehouse: 10k rows/1 link, 1299ms -> 672ms (essentially the fixed-8 number, since the budget granted full parallelism); 300k rows/19 links, 8552ms baseline vs 8561ms budgeted (neutral, as designed) vs 15068ms fixed-8 (the regression this design specifically avoids). The real file size is read from the first Range response's ownContent-Rangeheader, not the Thrift link'sbytesNum(confirmed to be the uncompressed row-set size, not the file's actual size on blob storage -- using it producesHTTP 416). A206response whoseContent-Rangecan't be parsed is treated as a hard failure, not a silent truncation to just the first probed part -- unreached against real Azure Blob Storage (always well-formed) but a third-party response shape this crate doesn't control, so failing loud beats guessing.thrift_pool_exhaustion_falls_back_to_a_throwaway_session_that_still_succeeds's poll-first pattern (see the entry above) already covers this change too. This is Thrift-only for now -- SEA's ownfetch_chunks_with_backpressurehas its own, separate concurrency model and doesn't route throughdownload_slots. -
execute_lazy_thriftnow callsDbClient::ensure_warehouse_runningbefore touching a session, matching SEA's ownsubmit_and_poll(which has always called it). Found during the same round of investigation as the entries above, flagged as a reliability gap rather than a perf one:ensure_warehouse_runningis plain REST against/api/2.0/sql/warehouses/{id}, nothing SEA- or Thrift-specific about it, but the Thrift path never called it -- a stopped warehouse got no proactive wake-up and nowarehouse_start_timeoutwait for it to come up onprotocol="thrift"(now the default), unlike SEA, which has always had this. Fixed by making the methodpub(crate)(was private, only reachable fromclient.rs's ownsubmit_and_poll) and calling it as the first line ofexecute_lazy_thrift. Every Thrift mock test needed a warehouse-status route added as a result -- bundled intomount_open_session_always/ThriftMockServer.do_GET's built-in routes (both Rust and Python) rather than repeated per test, since essentially every test that submits a Thrift statement needs it now, same as SEA's ownmock_warehouse/install_mock_warehousealready provide for that side. -
SEA's own
fetch_chunks_with_backpressure(client.rs) had the identical "server can over-deliver past its declared row count" gap the Thrift entry above describes -- found on a deliberate audit pass extending that same scrutiny to SEA, not from an observed real-warehouse failure. EveryChunkItemit built hardcodedtruncate_to: None, even thoughmeta.row_count(the manifest's declared per-chunk_indexcount) was right there, unused for this. Tested directly against the real warehouse first, not assumed:LIMIT 500000and a smallerWHERE-filteredLIMIT 5000both came back exactly right on SEA across repeated runs, so this gap hasn't been observed to actually bite -- unlike Thrift's version of the same bug, which measurably did (502,879 vs 500,000 rows). Fixed anyway, defensively: cheap (decode_chunk_item's truncation is already a no-op when the count already matches), matchesdatabricks-sql-connector's ownResultSetDownloadHandlerapplying this unconditionally regardless of protocol, and there's no principled reason SEA's cloud-fetch chunk generation is exempt from a behavior that's fundamentally about cloud-fetch, not the wire protocol on top of it. One real subtlety: achunk_indexcan resolve to more than one blob (ChunkMeta::pre_resolved_links, a real if rare/defensive-coding shape -- see its own doc comment), andmeta.row_countis the declared total for the whole chunk_index, not per-blob -- truncating each blob independently to that same total would be wrong when there's more than one.truncate_tois only set when there's exactly one blob (the common, unambiguous case); a chunk resolving to multiple blobs is left untruncated, same as before this fix. Seelazy_pipeline_truncates_a_sea_chunk_that_overshoots_its_declared_row_count.- The same audit found the identical gap a third time, in
run_json_pipeline'sdecode_json_chunk-- fixed the same way. A JSON_ARRAY chunk is delivered through the exact same cloud-fetch blob-storage mechanism as an Arrow one (samefetch_chunks_with_backpressure, sameChunkItem, so it already carries the correcttruncate_toafter the fix above), just decoded as a flat JSON array of rows instead of Arrow-IPC --decode_json_chunksimply never looked attruncate_toat all before this. No batch-boundary complexity here (unlike Arrow'sRecordBatch-slicing case): rows are already a flatVec<Vec<Option<String>>>, so it's a plainVec::truncate.stream_query_json(which shares the samefetch_chunks_with_backpressure/ChunkItemplumbing viaNdjsonStream) got this fix for free, no separate change needed. Verified against the real warehouse:stream_query_jsonon a 500k-row query yields exactly 500,000 items. Seejson_pipeline_truncates_a_chunk_that_overshoots_its_declared_row_count. - Volume files (
upload_volume_file/delete_volume_file) were checked for the same bug class and are genuinely exempt, not just untested. Plain PUT/DELETE against a fixed REST endpoint, no manifest, no chunking, no "declared count vs. what actually got decoded" concept to begin with -- there's nothing here for this bug class to apply to. - A real A/B is what caught (and then ruled out) an apparent regression from these two fixes -- worth recording the method, not just the conclusion. An interleaved benchmark right after the SEA fix showed arrowbricks winning only 9/13 and then 6/13 rounds against the connector, down from this session's earlier consistent 13-14/13-14 -- looked like a real regression at first glance. Isolated it properly: toggled
truncate_toback to unconditionalNonein an uncommitted local edit, rebuilt, and ran the same benchmark twice on each side of the toggle rather than trusting a single before/after pair. Pre-fix itself swung from 723ms mean/12-13 wins to 884ms mean/6-13 wins across two runs of identical code -- a bigger swing than the ~50ms difference between the pre-fix and post-fix aggregates (~803ms vs ~855ms, statistically indistinguishable given that noise floor). Confirms this test machine's network variance (already documented elsewhere in this file) is large enough to produce an apparent "regression" on completely unchanged code -- a single interleaved run is not enough to trust a perf delta this small; run it more than once before concluding anything moved.
- The same audit found the identical gap a third time, in
-
Cursor.fetchall_streamed()/fetchall_arrow_streamed()(cursor.py) now delegate to._core.ResultSet.fetchall_arrow_streamed(the Rust-level,heartbeat::HeartbeatWait-backed method) instead of wrappingself.fetchall()/self.fetchall_arrow()in this package's own Python-levelawait_with_heartbeat. Found in review (twice -- flagged once, "fixed" only in prose the first time, actually fixed the second): the two named trigger points the cancellation/on_eventdesign plugs into (heartbeat.rs'stick()/Drop for HeartbeatWait/Drop for HeartbeatStream) were real and fully wired, but the most common Python-facing timeout API never reached them at all, since it used a completely separate Python-level heartbeat with no cancellation hook. Fixed by routing both methods through._core.ResultSet.fetchall_arrow_streamed--fetchall_arrow_streamedyields it directly (translating_core.HEARTBEATto this package's ownHEARTBEATsingleton, and the Rust-level timeout's plainRuntimeError--heartbeat.rs's literal"Query exceeded {secs}s timeout", matched by that stable prefix, not guessed at -- into this package's ownQueryTimeout, preserving the exception type existing callers already rely on), andfetchall_streamed(row tuples) layers on top of it, draining any rowsfetchone()already buffered first (synchronously, before the heartbeat starts) the same wayfetchall()does. Both methods stay fully lazy --_require_empty_row_buffer/_require_resultare checked inside the returned generator, not when the method is called, matchingawait_with_heartbeat's own original laziness. Seetests/test_observability.py'stest_cursor_fetchall_arrow_streamed_total_timeout_fires_server_side_cancel/test_cursor_fetchall_streamed_row_variant_also_raises_query_timeoutfor the regression coverage (both assert the mock cancel endpoint is actually hit, not just thatQueryTimeoutis raised).- Still out of scope, on purpose, and sharing the identical blind spot on both features: a timeout/cancellation during the initial submit/poll wait (
Cursor.execute()/execute_streamed(),_streaming.py's ownawait_with_heartbeatwrappingcore_client.execute(...)) triggers neither a cancel RPC nor anon_eventdispatch -- there's noheartbeat.rswrapper on this phase at all (a documented, pre-existing quirk, see the "execute_streamed's heartbeat/timeout only covers the wait..." entry above), so nothing catches an abandonment there. In practice this matters less than it sounds: by the time any chunk is being fetched, the statement has normally already reached SUCCEEDED/FAILED server-side, so the two cancellation triggers that are wired (both fire only during the chunk-download phase) are mostly hitting an already-terminal statement anyway --CancelOperation/POST .../cancelagainst a finished statement is harmless (see the "belt-and-suspenders" reasoning below), just not the primary "stop a still-running query" win the phrase "cancellation" suggests.pipeline.rs'sreport_submit_errordoes cover the more common non-cancellation submit/poll failure (a FAILED statement, bad SQL, or a stopped/unreachable warehouse) with anon_eventoutcome of"error"-- but withstatement_id: "", sinceApiErrordoesn't carry it back fromclient.rs.
- Still out of scope, on purpose, and sharing the identical blind spot on both features: a timeout/cancellation during the initial submit/poll wait (
-
QueryStatsAccumulator(client.rs) is a small set of atomics (bytes_downloaded,retry_count,chunks_seen) threaded explicitly through every retryable network call and cloud-fetch download on a query's own path --retry_call/authed_json/thrift_call/fetch_link_bytes*all gained anOption<&QueryStatsAccumulator>/&QueryStatsAccumulatorparameter for this. Deliberately not gated behind "only ifon_eventis set" -- a few atomic increments per request is cheap enough to always do, soon_event.is_none()only skips the final dispatch, not the accumulation. If you add a new retryable call that should count toward a query'sretry_count/bytes_downloaded, thread the accumulator to it the same way; calls that are deliberately not query-scoped (session create/close,upload_volume_file/delete_volume_file, the cancel RPC itself) intentionally passNone/skip it, since attributing their retries to one specific query would be arbitrary (a pooled session can outlive and be reused by several).chunks_seenis only actually incremented by the Thrift path (run_thrift_fetch_loop) -- SEA's ownQueryStats.num_chunkscomes from the already-knownchunk_metas.len()instead (StatsReporter::static_num_chunks), since SEA's whole manifest is known upfront and Thrift's isn't. Don't "simplify" this into incrementingchunks_seenunconditionally for both protocols without checkingStatsReporter::finish's own fallback logic first.warehouse_wait_sis timed and recorded exactly once, insideclient.rs'ssubmit_and_poll/pipeline.rs'ssubmit_thrift_and_start_fetch(viaQueryStatsAccumulator::add_warehouse_wait_s), never externally by apipeline.rscall site. Found in review: an earlier version hadexecute_lazy/execute_lazy_prefer_inline/execute_ndjson_stream's SEA branch each time their own call toensure_warehouse_runningbefore callingexecute_arrow_statement*-- which callssubmit_and_poll, which callsensure_warehouse_runningagain, unconditionally. Not just three copies of the same boilerplate (seesubmit_sea_and_report, added to collapse them into one shared helper): a genuine double network-adjacent call per query (the second one cache-warm and cheap, but still a real, pointless second call), and the external copy's own error path didn't callreport_submit_error, so a stopped/unreachable warehouse silently never firedon_eventat all despitereport_submit_error's own doc comment claiming to cover it.add_warehouse_wait_saccumulates rather than overwrites specifically becauseexecute_arrow_statement_prefer_inline's recognized byte-limit fallback (client.rs) can meansubmit_and_pollruns twice for one logical query -- summing gives the true total wait, not just the second (typically ~0, cache-warm) call's.execute_lazy_prefer_inlinereports JSON-conversion failure through the samestats/submit_t0as the succeeded INLINE attempt. The error path preserves the attempt's timing and counters for itson_eventerror outcome, then returns the conversion error without submitting another statement. The separate recognized byte-limit fallback inexecute_arrow_statement_prefer_inlinereusesstatsand is timed as one unbroken span frompipeline.rs.- Cancellation's own outcome hint (
QueryStatsAccumulator::store_outcome_if_unset, distinguishing"timeout"from"cancelled") must be set beforeheartbeat.rs'stick()/Dropcallhandle.abort(), not after -- by the time the aborted task's ownDropimpls run (pipeline.rs'sPoisonOnDrop/ReportOnDrop, which read this hint to decide what to report), the closure that set it may already be unreachable if it ran any later.pipeline::cancel_hook(notlib.rs) builds this closure specifically so it's constructible -- and its ordering testable -- from a plain#[tokio::test]with no PyO3 involved (seewiremock_pipeline.rs/wiremock_thrift.rs's own cancellation tests, which constructHeartbeatWait/HeartbeatStreamdirectly with.with_cancel(pipeline::cancel_hook(...))).
-
EventSink/QueryStatsAccumulator/CancelHandlemirror the existingTokenProvider/PyTokenProvidersplit exactly on purpose: the trait + plain data (client.rs, no PyO3) vs. the PyO3-specific bridging (PyEventSink,lib.rs) -- seePyEventSink's own doc comment for the one deliberate difference (on_eventis dispatched fire-and-forget via a spawned task, never awaited inline, unliketoken_provider) and its documented residual limitation (an asyncon_eventfired from theDrop-triggered abandonment path has no asyncio event-loop context of its own to captureTaskLocalsfrom, the same root causePyTokenProvider's own doc comment describes for chunk-fetch worker tasks -- falls back to whatever an earlier successful dispatch already cached, so a client's very first query being the one that gets cancelled/timed out, with an async callback, may silently fail to dispatch; a sync callback has no such gap). -
PoisonOnDrop/ReportOnDrop(pipeline.rs) each have afail<T>(&mut self, e: ApiError) -> Result<T, ApiError>method -- reportsoutcome="error"and handseback wrapped, so a call site writesreturn guard.fail(e);instead of repeatingguard.reporter.finish("error", guard.stats); return Err(e);at every error-producing point (fetch_at_least/next_chunkeach had this duplicated 3x before).submit_thrift_and_start_fetchreturns a namedThriftSubmitResultstruct, not a positional tuple, for the same "found duplicated/error-prone in review" reasons (a 7-element tuple needing#[allow(clippy::type_complexity)], destructured-then-reassembled by both its callers, with two same-typedf64timing fields one reorder away from silently swapping).tests/common/mod.rs(amod common;in bothwiremock_pipeline.rs/wiremock_thrift.rs, not itself auto-discovered as a test binary -- the standard Rust integration-test idiom) holdswait_for_calls, previously duplicated verbatim in both files' cancellation tests. -
ApiErrorcarries akind: ApiErrorKindfield (Other/Auth/Statement) alongside its existingtransient: bool, and everyApiError->PyErrconversion inlib.rsgoes through one mapping function (api_error_to_pyerr), notPyRuntimeError::new_err(e.message)at each of the ~15 call sites that used to do it independently (2026-08-11).kindis set at exactly three places:ApiError::from_statussetsAuthfor HTTP 401/403 (the same statuses that were already unconditionallytransient, since a retry there re-fetches a token --kinddoesn't change retry behavior, only which Python exception type fires once retries are exhausted);ApiError::statement_failed(a new constructor, alongside the existingApiError::permanent) setsStatement, used only where a Databricks statement/operation reached a real terminal FAILED/CANCELED/error state -- SEA'ssubmit_and_poll_innerFAILED/CANCELED arms, and Thrift's twoterminal_error()call sites inpipeline.rs'ssubmit_and_await_thrift_statement; andpy_err_to_api_error(wraps aPyErrraised by the caller's owntoken_provider) also setsAuthunconditionally -- see its own doc comment and the entry below for why. Deliberately not used for a Thrift RPC's own transport-levelTStatuserror (e.g.FetchResultsreturningINVALID_HANDLE) -- that's a protocol/transport problem, not necessarily evidence the statement itself failed, so those stayApiError::permanent/kind: Other. Every otherApiErrorconstruction site (parse/decode/internal-invariant errors --json_convert.rs,pipeline.rs's Arrow-IPC/NDJSON encode paths,heartbeat.rs's timeout error) is unclassified (Other), whichapi_error_to_pyerrmaps to the plainArrowbricksErrorbase if non-transient orTransientErrorif transient --Otheris the default via#[derive(Default)]onApiErrorKind, but every literalApiError { .. }construction site still spells outkind: ApiErrorKind::Otherexplicitly rather than relying on struct-update syntax, so a future new field onApiErrorcan't silently default itself in at a site that should have picked something else.py_err_to_api_error(thetoken_provider-failure wrapper) originally leftkindas theOtherdefault too -- found in independent code review, not by any test in the first draft, and fixed the same session. This meant atoken_providerthat itself raised (e.g. its own OAuth refresh call came back unauthorized) never surfaced asAuthError-- exactly the case README.md'sexcept AuthError: refresh_credentials()pattern most wants to catch, since a broken/expired credential is precisely what a caller's own token provider is most likely to fail on. Fixed by classifying everyPyErrreachingpy_err_to_api_errorasAuth, justified purely by that function's calling context (its only caller isPyTokenProvider::get_token, so everyPyErrit wraps happened while specifically trying to obtain a bearer token) rather than by inspecting the exception object itself -- there is no reliable way to tell "the token provider raised an auth-specific error" from "it raised some other exception" from thePyErralone, since a caller'stoken_providercan raise literally anything.transientstaysfalsehere, unchanged -- a brokentoken_providerisn't retried byretry_call_trackedthe way a transient HTTP status is, and that's a separate question fromkind's own classification. Seetests/test_errors.py'stest_token_provider_raising_surfaces_as_auth_error.- The four Python-facing exception types (
ArrowbricksError,TransientError,AuthError,StatementError) are defined via PyO3'screate_exception!macro inlib.rs, not as plain Python classes in a.pymodule.create_exception!(_core, Name, Base, "doc")registers a real CPython exception type with the given base (chaining works --TransientError/AuthError/StatementErrorall specifyArrowbricksError, itself specifyingpyo3::exceptions::PyRuntimeError, as their base) and needs onem.add("Name", m.py().get_type::<Name>())line per type in the_coremodule-init function to actually make it importable. Chosen over defining these in Python and reaching for them from Rust viapy.import("arrowbricks._errors")?.getattr(...)on every error: no per-error Python-level module lookup, and the type is real enough forisinstance/exceptto work exactly like a built-in exception.ArrowbricksErrorsubclassesPyRuntimeError, notPyException-- deliberate backward compatibility: every exception this package raises itself used to be a plainRuntimeError, and anexcept RuntimeErrorwritten before this hierarchy existed must keep working unchanged.QueryTimeout(_streaming.py, still Python-defined and raised, not one of thecreate_exception!types -- seecursor.py's ownRuntimeError-message-prefix-match translation ofheartbeat.rs's timeout error) was changed to additionally subclass_core.ArrowbricksErrorfor the same reason, soexcept ArrowbricksErroris a genuine single catch-all across every exception this package raises, timeout included. - See
tests/test_errors.py(401/403 ->AuthError, FAILED/CANCELED ->StatementError, a persistent 429 ->TransientError, a generic 400 -> the plainArrowbricksErrorbase, and an explicitisinstance(..., RuntimeError)assertion on all of them) for the covering tests, run through the real publicCursor/DatabricksClientAPI, not just Rust unit tests.
-
proptest(dev-only, added 2026-08-11) fuzzes this crate's hand-rolled wire-protocol parsers -- and immediately found a real, previously-unknown stack-overflow bug, not just a theoretical exercise.thrift.rs'sReader::skip(generically skips any field a<Struct>::readmethod doesn't recognize -- see its own doc comment) recursed into itself for nestedSTRUCT/LIST/SET/MAPfields with no depth bound at all.thrift::proptests::skip_never_stack_overflows_on_deeply_nested_structs(a targeted property -- plain random bytes essentially never produce thousands of genuine nesting levels in a row, since each level needs an exact 3-byte field header out of 256^3 possibilities) constructed a small (~12KB), well-formed-looking buffer of a few thousand nestedSTRUCTfield headers and crashed the whole test process with a hard stack-overflow abort (SIGABRT, "has overflowed its stack") -- not a catchable panic, since Rust's stack overflow can't be turned into a Python exception bycatch_unwindat the PyO3 boundary the way a normal panic can. A corrupted or malicious Thrift response (this crate's own field-1281-vs-12 bug above shows real-world Thrift field handling has already had at least one genuine mismatch with the spec) could in principle take down an entire long-running server process this way, not just fail one query. Fixed withReader::MAX_SKIP_DEPTH(64 -- generous headroom past any real struct this crate parses, which nests at most a handful of levels) threaded through a newskip_bounded(ftype, depth)thatskipcalls withdepth: 0; every recursive call incrementsdepthand errors cleanly once it hits the cap instead of recursing further. Seethrift.rs's ownMAX_SKIP_DEPTHdoc comment and the hand-written regression testskip_rejects_nesting_past_max_skip_depth_but_allows_shallow_nesting(proves both directions: shallow/real nesting still skips fine, and nesting past the cap gets a cleanErr, not a crash).- Every other property test added in this same pass (
thrift::proptests'parse_functions_never_panic_on_arbitrary_bytes/reader_field_loop_never_panics_on_arbitrary_bytes/operation_handle_round_trips_through_write_then_read,json_convert::proptests' fuzzing ofparse_struct_fields/parse_one_field/base64_decode/parse_decimal_to_i128/build_column,client::proptests' fuzzing ofdecompress_lz4_frameincluding a truncated real frame variant) passed clean on the existing code -- i.e. the rest of this hand-rolled parsing surface's existing bounds-checking (Reader::take's check-before-slice, the empty-blob rejection ondecode_chunk, etc.) held up under fuzzing, not just manual/real-workspace testing. Worth knowing for the next person tempted to assume fuzzing this code will always find something: it found exactly one bug, in exactly the one place (skip's unbounded recursion) that had never been exercised by anything before, real workspace included -- every other real Thrift response this crate has ever parsed happens to nest shallowly. - Runs inside plain
cargo test(proptest!macro expands to normal#[test]functions under the hood) -- no separate fuzz-target/corpus infra, no nightly toolchain, unlikecargo-fuzz/afl. Default case count (256/property) is left alone; adds ~0.1s to the wholecargo test --no-default-featuresrun (12 proptests, 1.0s total lib-test time including everything else) -- not a meaningful slowdown.[dev-dependencies]-only, confirmed not to touch the shipped wheel's dependency graph (dev-dependencies never link into thecdylibmaturin builds, only intocargo test/cargo benchbinaries).
- Every other property test added in this same pass (
tests/conftest.py's MockServer spins up a real local ThreadingHTTPServer per test (via the mock_warehouse/mock_volume_files/mock_server fixtures) -- both client.py and ._core's reqwest calls hit the same real socket, since respx (transport-level httpx mocking) can't see reqwest's requests at all and client.py has no httpx of its own anymore anyway. Routes register respx-style (server.get(path).mock(Response(...)), regex paths via regex=True, sequential responses via side_effect=[...], dynamic responses via a callable). Pass reverse_arrival=True to mock_warehouse to force genuine out-of-order chunk completion when a test needs to prove ordering survives it.
protocol="thrift" tests use a parallel mock, tests/conftest.py's mock_thrift_server fixture (ThriftMockServer from tests/thrift_mock.py, duplicated self-contained in rust/arrowbricks_core/tests_py/thrift_mock.py -- see the design-invariant entry above for why) -- also a real local ThreadingHTTPServer, but routed differently: every Thrift RPC hits the one shared path, so server.handler.<rpc_name> = lambda req: ... assigns a per-RPC callable (a real, already-parsed request object in, a real response object out) instead of registering a (method, path) route. rust/arrowbricks_core/tests/wiremock_thrift.rs is the Rust-side counterpart, over wiremock -- IsThriftRpc, a custom matcher parsing the Thrift message name out of the request body, does the analogous routing there.
Cancellation tests (wiremock_pipeline.rs's sea_total_timeout_fires_cancel_statement/sea_dropping_the_heartbeat_wait_mid_fetch_fires_cancel_statement, wiremock_thrift.rs's Thrift-CancelOperation counterparts) construct heartbeat::HeartbeatWait/HeartbeatStream directly with .with_cancel(pipeline::cancel_hook(...)) against a deliberately slow (500ms) single-chunk mock response, rather than going through execute_streamed/Cursor -- the actual cancel-firing mechanism lives in pipeline::cancel_hook, reachable from a plain #[tokio::test] with no PyO3 needed (see the design-invariant entry above for why this isn't tested through lib.rs/Python instead). The cancel RPC/REST call is fire-and-forget (spawned, not awaited), so these tests poll (wait_for_calls) rather than asserting immediately after the timeout/drop. tests/test_observability.py covers QueryStats/on_event end to end through the real Python API (DatabricksClient/Cursor/stream_query_json) instead, including one test that hits a real mock POST .../cancel endpoint through the full stack (Python -> ._core.Client.stream_ndjson_lines -> HeartbeatStream -> pipeline::cancel_hook).
- Bump
versioninpyproject.toml(andrust/arrowbricks_core/Cargo.toml, kept in step for clarity even though only the root version ends up in the published wheel's metadata). git tag vX.Y.Z && git push origin vX.Y.Z..github/workflows/release.ymlruns the Rust+Python test job, then builds cross-platform wheels (Linux x86_64/aarch64, macOS x86_64/aarch64, Windows x64) + an sdist viamaturin-action, and publishes to PyPI via trusted publishing (OIDC) -- no stored token. abi3-py311 means one wheel per (OS, arch) covers every supported Python, no per-version build matrix.
One-time, outside this repo: register this GitHub repo + release.yml workflow as a trusted publisher on the arrowbricks PyPI project (PyPI project settings -> Publishing). Without that, the publish job's OIDC exchange fails even though tests pass.