[SPARK-58024][PYTHON] Convert Arrow struct and map columns to Python rows in bulk#57105
Closed
viirya wants to merge 2 commits into
Closed
[SPARK-58024][PYTHON] Convert Arrow struct and map columns to Python rows in bulk#57105viirya wants to merge 2 commits into
viirya wants to merge 2 commits into
Conversation
8b51d7a to
1cf3d8a
Compare
1cf3d8a to
f3340f4
Compare
904fa86 to
96a606f
Compare
…rows in bulk Extend ArrowTableToRowsConversion._to_pylist with bulk paths for struct and map columns: * Struct columns convert each child field in bulk (recursively reusing the list/leaf fast paths), then zip the field values into one dict per row, masked by the validity bitmap. Duplicate field names fall back to to_pylist so they raise the same ValueError that StructScalar.as_py raises. * Map columns share the list offsets layout: the flattened keys and items children are each converted in bulk and every row becomes a list of (key, value) tuples, matching MapScalar.as_py exactly. ASV microbenchmark (bench_arrow.ArrowStructMapColumnToRowsBenchmark, 1M rows, 10% nulls): struct<int64,string> 914ms -> 175ms (5.2x); map<string,int64> with 2 entries per row 2.07s -> 303ms (6.8x). Peak memory unchanged. Co-authored-by: Isaac
96a606f to
40f92f7
Compare
Member
Author
|
@huaxingao Can you hold 4.2 new RC cut until this gets merged? This is to extend #57099 to struct and map columns. Thanks. |
Member
Author
|
@dongjoon-hyun @Yicong-Huang @gaogaotiantian This extends the optimization to struct and map columns. Could you also take a look? Thanks! |
| for i in range(n) | ||
| ] | ||
|
|
||
| if pa.types.is_list(column.type) or pa.types.is_large_list(column.type): |
Contributor
There was a problem hiding this comment.
nit: could you change this if and the one below to elif? otherwise looks good to me.
Co-authored-by: Isaac
gaogaotiantian
approved these changes
Jul 11, 2026
viirya
added a commit
that referenced
this pull request
Jul 11, 2026
…rows in bulk ### What changes were proposed in this pull request? Follow-up of SPARK-58019 (#57099); **only the last commit is new — this PR is stacked on #57099 and will be rebased once it merges.** (Originally stacked on SPARK-58023 (#57104), which has been closed in favor of the upstream apache/arrow#50326 fix; the numbers below are measured without it.) Extend `ArrowTableToRowsConversion._to_pylist` with bulk paths for struct and map columns: - **Struct** columns convert each child field in bulk (recursively reusing the bulk list paths), then zip the field values into one dict per row, masked by the validity bitmap. Duplicate field names fall back to `to_pylist`, so they raise the same `ValueError` that `StructScalar.as_py` raises. - **Map** columns share the list offsets layout: the flattened keys and items children are each converted in bulk, and every row becomes a list of `(key, value)` tuples, matching `MapScalar.as_py` exactly. ### Why are the changes needed? Struct and map columns still convert one Scalar per row; per-row Scalar/wrapper allocation dominates (apache/arrow#50326), and maps are the worst case since every row also wraps its keys/items in per-row Arrays. ASV microbenchmark (`bench_arrow.ArrowStructMapColumnToRowsBenchmark`, 1M rows, 10% nulls, PyArrow 24.0.0): | case | `to_pylist()` | this PR | speedup | |---|---|---|---| | `struct<int64,string>` | 970 ms | 474 ms | 2.0x | | `map<string,int64>` (2 entries/row) | 2.16 s | 799 ms | 2.7x | Peak memory unchanged. ### Does this PR introduce _any_ user-facing change? No. Only performance; conversion results are identical (covered by exact-type tests). ### How was this patch tested? Extended `ArrowColumnToPylistTests` with struct (incl. nested struct/list children, empty struct, all-null), map (incl. list values), struct-of-map, list-of-struct, sliced and chunked views — all compared against `column.to_pylist()` with exact element-type assertions — plus dedicated tests that duplicate struct field names still raise `ValueError` and that empty-struct rows are distinct dict objects. New ASV benchmark class `ArrowStructMapColumnToRowsBenchmark`. ### Was this patch authored or co-authored using generative AI tooling? Yes. This pull request and its description were written by Isaac (Claude Code). Closes #57105 from viirya/arrow-to-pylist-maps-structs. Authored-by: Liang-Chi Hsieh <viirya@gmail.com> Signed-off-by: Liang-Chi Hsieh <viirya@gmail.com> (cherry picked from commit 9c6e57b) Signed-off-by: Liang-Chi Hsieh <viirya@gmail.com>
viirya
added a commit
that referenced
this pull request
Jul 11, 2026
…rows in bulk ### What changes were proposed in this pull request? Follow-up of SPARK-58019 (#57099); **only the last commit is new — this PR is stacked on #57099 and will be rebased once it merges.** (Originally stacked on SPARK-58023 (#57104), which has been closed in favor of the upstream apache/arrow#50326 fix; the numbers below are measured without it.) Extend `ArrowTableToRowsConversion._to_pylist` with bulk paths for struct and map columns: - **Struct** columns convert each child field in bulk (recursively reusing the bulk list paths), then zip the field values into one dict per row, masked by the validity bitmap. Duplicate field names fall back to `to_pylist`, so they raise the same `ValueError` that `StructScalar.as_py` raises. - **Map** columns share the list offsets layout: the flattened keys and items children are each converted in bulk, and every row becomes a list of `(key, value)` tuples, matching `MapScalar.as_py` exactly. ### Why are the changes needed? Struct and map columns still convert one Scalar per row; per-row Scalar/wrapper allocation dominates (apache/arrow#50326), and maps are the worst case since every row also wraps its keys/items in per-row Arrays. ASV microbenchmark (`bench_arrow.ArrowStructMapColumnToRowsBenchmark`, 1M rows, 10% nulls, PyArrow 24.0.0): | case | `to_pylist()` | this PR | speedup | |---|---|---|---| | `struct<int64,string>` | 970 ms | 474 ms | 2.0x | | `map<string,int64>` (2 entries/row) | 2.16 s | 799 ms | 2.7x | Peak memory unchanged. ### Does this PR introduce _any_ user-facing change? No. Only performance; conversion results are identical (covered by exact-type tests). ### How was this patch tested? Extended `ArrowColumnToPylistTests` with struct (incl. nested struct/list children, empty struct, all-null), map (incl. list values), struct-of-map, list-of-struct, sliced and chunked views — all compared against `column.to_pylist()` with exact element-type assertions — plus dedicated tests that duplicate struct field names still raise `ValueError` and that empty-struct rows are distinct dict objects. New ASV benchmark class `ArrowStructMapColumnToRowsBenchmark`. ### Was this patch authored or co-authored using generative AI tooling? Yes. This pull request and its description were written by Isaac (Claude Code). Closes #57105 from viirya/arrow-to-pylist-maps-structs. Authored-by: Liang-Chi Hsieh <viirya@gmail.com> Signed-off-by: Liang-Chi Hsieh <viirya@gmail.com> (cherry picked from commit 9c6e57b) Signed-off-by: Liang-Chi Hsieh <viirya@gmail.com>
Member
Author
Member
Author
|
Thanks @gaogaotiantian |
Member
Author
|
@huaxingao Both PRs are now merged into master: #57099 (SPARK-58019) and this #57105 (SPARK-58024). Nothing else is blocking on our side, so please feel free to proceed with the new 4.2 RC cut. Thank you for your patience and for holding the RC for these! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Follow-up of SPARK-58019 (#57099); only the last commit is new — this PR is stacked on #57099 and will be rebased once it merges. (Originally stacked on SPARK-58023 (#57104), which has been closed in favor of the upstream apache/arrow#50326 fix; the numbers below are measured without it.)
Extend
ArrowTableToRowsConversion._to_pylistwith bulk paths for struct and map columns:to_pylist, so they raise the sameValueErrorthatStructScalar.as_pyraises.(key, value)tuples, matchingMapScalar.as_pyexactly.Why are the changes needed?
Struct and map columns still convert one Scalar per row; per-row Scalar/wrapper allocation dominates (apache/arrow#50326), and maps are the worst case since every row also wraps its keys/items in per-row Arrays. ASV microbenchmark (
bench_arrow.ArrowStructMapColumnToRowsBenchmark, 1M rows, 10% nulls, PyArrow 24.0.0):to_pylist()struct<int64,string>map<string,int64>(2 entries/row)Peak memory unchanged.
Does this PR introduce any user-facing change?
No. Only performance; conversion results are identical (covered by exact-type tests).
How was this patch tested?
Extended
ArrowColumnToPylistTestswith struct (incl. nested struct/list children, empty struct, all-null), map (incl. list values), struct-of-map, list-of-struct, sliced and chunked views — all compared againstcolumn.to_pylist()with exact element-type assertions — plus dedicated tests that duplicate struct field names still raiseValueErrorand that empty-struct rows are distinct dict objects. New ASV benchmark classArrowStructMapColumnToRowsBenchmark.Was this patch authored or co-authored using generative AI tooling?
Yes. This pull request and its description were written by Isaac (Claude Code).