Remove Empty Chunks in Cloudfetch concatenation - #814
Conversation
Sorry, something went wrong.
|
P2 — Minor
fetchall_arrow uses partial_result_chunks: List = [] while fetchmany_arrow:313 uses List["pyarrow.Table"]. Cosmetic; align both.
If a pathological server returned 0-row chunks while leaving has_more_rows=True forever, both methods spin. The old code (append-without-decrement) and the new code (continue) have identical semantics here.
All four new tests place the placeholder first. The continue path is exercised, but only with the initial queue being the placeholder — there's no [real, placeholder, real] test that locks in the mid-stream
_StubArrowQueue returns the full table on first call and slice(0, 0) thereafter. Production next_n_rows(N) returns up to N rows. The tests happen to call fetchmany_arrow(3) against a 3-row table so the |
Sorry, something went wrong.
gopalldb
left a comment
There was a problem hiding this comment.
Added some minor comments
Sorry, something went wrong.
Comments regarding the tests are fine, but our main point is to check whether concatenation is averted or not with the empty chunks and this works, so its fine |
Sorry, something went wrong.
What type of PR is this?
Description
ThriftResultSet.fetchmany_arrow and fetchall_arrow previously appended every chunk
returned by the underlying queue — including the 0-row placeholder that
CloudFetchQueue._create_empty_table() emits when self.table is None — into
partial_result_chunks, and then handed the list to concat_table_chunks.
When the placeholder's schema differs from the real downloaded chunks (which it can,
because it is built from schema_bytes that may be stale, or schemaless when
schema_bytes is None), pyarrow.concat_tables(..., promote_options="default")
silently introduces phantom columns filled with NULLs, or — for type mismatches —
raises.
Fix: hold any 0-row chunk aside in a local zero_row_table instead of appending it.
The concat list now only ever contains real chunks, which share a consistent schema.
If every chunk turned out to be 0-row (genuinely empty result set), fall back to
appending the held-aside placeholder so the method still returns a valid pyarrow.Table.
CloudFetchQueue itself is unchanged. The columnar fetch paths are unchanged
(they only run with ColumnQueue, whose empty slices always carry the right schema).
The SEA arrow methods are unchanged (single queue call per invocation, no concat).
How is this tested?
Added 4 regression tests in tests/unit/test_fetches.py:
0-row placeholder with column stale_col; second returns real data with col0.
Asserts the result has only col0. Verified to fail against the pre-fix code with
['stale_col', 'col0'] != ['col0'].
asserts a pyarrow.Table with num_rows == 0 is returned (the held-aside fallback fires).
Full unit suite (pytest tests/unit -x): 765 passed, 4 skipped.
Integration / e2e to be run separately.