[SPARK-58019][PYTHON] Convert Arrow list columns to Python rows in bulk#57099
[SPARK-58019][PYTHON] Convert Arrow list columns to Python rows in bulk#57099viirya wants to merge 5 commits into
Conversation
PyArrow's Array.to_pylist() materializes one Scalar per element; for list-typed columns each row additionally allocates a C++ scalar, a Python Scalar wrapper and a Python Array wrapper before converting elements one by one. This makes Arrow-optimized Python UDF inputs and Spark Connect collect() several times slower on array columns than necessary (apache/arrow#50326; a fix is proposed upstream in apache/arrow#50327 but will only be available in a future PyArrow release). Add ArrowTableToRowsConversion._to_pylist, which converts the flattened child values of a list column in a single pass and slices the resulting Python list per row using the offsets and the validity bitmap, and use it in the Arrow-to-rows conversion paths (Spark Connect collect, Arrow batch UDF inputs, Arrow UDTF inputs). Leaf values are still converted by Arrow's own to_pylist, so results are exactly identical - None stays None and values inside numeric lists stay Python ints, unlike a pandas round trip which coerces them to floats/NaN. NumPy is only used for the offsets and validity bitmap, never for the values. ASV microbenchmark (python/benchmarks/bench_arrow.py, 1M rows): list<string> 769ms -> 507ms (1.5x); list<list<int32>> with nulls 1.86s -> 537ms (3.5x). Peak memory unchanged. The helper can be removed once the minimum supported PyArrow version includes the upstream fix. Co-authored-by: Isaac
|
cc @Yicong-Huang FYI |
dbtsai
left a comment
There was a problem hiding this comment.
Reviewed the full diff. Nice, well-scoped optimization. I traced the correctness-critical _to_pylist path against to_pylist() semantics and it holds up on the edges that matter:
- Sliced arrays:
column.valuesreturns the full unsliced child andcolumn.offsetsare absolute into it, so readingstart = offsets[0], slicingvalues.slice(start, offsets[-1]-start), and rebasing every per-row slice by-startis correct. Using.valuesrather than.flatten()is the right call here --.flatten()would drop null-slot ranges and break the rebasing. - Null slots: Arrow does not guarantee a null list slot has an empty offset range, and keying None-ness off the validity bitmap (
valid[i]) rather than off an empty range handles that correctly. - Exact types: leaf values still go through Arrow's own
to_pylist; NumPy only touches offsets and the validity bitmap, solist<int32>[1, None, 3]stays ints -- the pandas round-trip float/NaN coercion is genuinely avoided. - Fallbacks: map / fixed_size_list / list_view don't match
is_list/is_large_listand fall through toto_pylist(). list_view is the important one (non-monotonic offsets would break the slicing), and it correctly does not take the fast path.
The test matrix -- comparing _to_pylist vs to_pylist() with exact element-type assertions across list/large_list/nested/struct/map/fixed_size, and across sliced and chunked views -- is exactly what's needed for a function that reimplements Arrow semantics.
Approve from my side. Two minor comments inline, both non-blocking.
| column | ||
| ) > 0: | ||
| n = len(column) | ||
| offsets = column.offsets.to_numpy(zero_copy_only=True).tolist() |
There was a problem hiding this comment.
Minor: zero_copy_only=True here is load-bearing and works only because list offset buffers never carry a validity bitmap. That invariant is correct, but non-obvious -- a one-line comment would stop a future reader from "fixing" it to zero_copy_only=False (and would flag the assumption if some new Arrow list variant ever violated it).
There was a problem hiding this comment.
Good point — added the comment in eebbc4f. zero_copy_only=True also doubles as an assertion: if a future Arrow list variant ever produced offsets with a validity bitmap, this would fail loudly instead of silently copying.
| if column.null_count == 0: | ||
| return [flat[offsets[i] - start : offsets[i + 1] - start] for i in range(n)] | ||
| valid = column.is_valid().to_numpy(zero_copy_only=False).tolist() | ||
| return [ |
There was a problem hiding this comment.
Minor (no change needed): the per-row slicing here is still O(rows) Python (offsets.tolist() + a comprehension slicing flat), so the win is fewer object allocations, not vectorization -- consistent with the benchmark numbers. Fine as the documented interim tradeoff; noting it so the "bulk" framing isn't read as fully vectorized.
There was a problem hiding this comment.
Right — row assembly is still an O(rows) Python loop; the win is that each row costs one list slice instead of a C++ scalar + Python Scalar wrapper + Python Array wrapper + generator. "Bulk" refers to the child-values conversion being a single call, not to vectorized row assembly. The fully vectorized fix is the upstream one (apache/arrow#50327); this stays intentionally simple as the interim.
Co-authored-by: Isaac
The files were formatted with black 26.3.1 (dev/requirements.txt), but the CI linter checks with ruff format, which disagrees on hunks unrelated to this change. Reformat with ruff 0.14.8 to match CI. Co-authored-by: Isaac
| @@ -844,6 +844,83 @@ def test_geometry_convert_numpy(self): | |||
| self.assertEqual(len(result), 0) | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
We need skipIf like the other arrow-based test cases.
| @unittest.skipIf(not have_pyarrow, pyarrow_requirement_message) |
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1, LGTM (with one comment about skipIf).
Co-authored-by: Isaac
…<struct> benchmarks Address review: cache the NumPy availability check in a module-level helper instead of re-running try/import on every (recursive) invocation, and extend the ASV benchmark with an array<struct> case plus peakmem variants for the nested cases (1M rows: nested list 862M -> 862M, array<struct> 885M -> 921M, ~4% peak increase from the transient flattened pointer list; leaf objects are shared). Co-authored-by: Isaac
What changes were proposed in this pull request?
Add
ArrowTableToRowsConversion._to_pylist, which converts Arrow list-typed columns to Python values in bulk: the flattened child values are converted with a single recursiveto_pylistcall, and the resulting Python list is sliced per row using the offsets and the validity bitmap. It is used in the Arrow-to-rows conversion paths (Spark Connectcollect(), Arrow batch UDF inputs, Arrow UDTF inputs). Non-list columns, map columns and environments without NumPy fall back to plaincolumn.to_pylist().Leaf values are still converted by Arrow's own
to_pylist, so results are exactly identical tocolumn.to_pylist():NonestaysNoneand values inside numeric lists stay Python ints. NumPy is used only for the offsets (non-null integers) and the validity bitmap (booleans), never for the values, so the type coercion problems of a pandas round trip (list<int32>of[1, None, 3]becoming[1.0, nan, 3.0]) cannot occur.This is an interim measure for a PyArrow-side inefficiency:
Array.to_pylist()materializes one Scalar per element, and for list types each row additionally allocates a C++ scalar, a Python Scalar wrapper and a Python Array wrapper before converting elements one by one (root-caused in apache/arrow#50326, fix proposed in apache/arrow#50327). The helper documents this and can be removed once the minimum supported PyArrow version includes the upstream fix.Why are the changes needed?
Converting Arrow list columns to Python rows is the hot path of Arrow-optimized Python UDF inputs and Spark Connect
collect(). With plainto_pylist()it is several times slower than necessary, which caused a performance regression on array columns when Arrow serialization became the default for regular Python UDFs.ASV microbenchmark (
python/benchmarks/bench_arrow.py::ArrowListColumnToRowsBenchmark, added in this PR; 1M rows, macOS arm64, PyArrow 24.0.0):to_pylist()list<string>list<list<int32>>with nullsPeak memory is unchanged.
Does this PR introduce any user-facing change?
No. Only performance; conversion results are byte-identical (covered by exact-type tests).
How was this patch tested?
New
ArrowColumnToPylistTestsinpython/pyspark/sql/tests/test_conversion.py, comparing_to_pylistagainstcolumn.to_pylist()with exact element-type assertions across list/large_list/nested/struct/map/fixed-size-list columns, sliced and chunked variants, plus a dedicated test thatlist<int32>of[1, None, 3]stays ints, and an end-to-endArrowTableToRowsConversion.converttest with array columns. Fulltest_conversion.pypasses. The new ASV benchmark class parametrizesbaselinevsbulk, so the comparison above is reproducible with./python/asv run --python=same --quick -b 'bench_arrow.ArrowListColumnToRowsBenchmark'.Was this patch authored or co-authored using generative AI tooling?
Yes. This pull request and its description were written by Isaac (Claude Code).