You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "viirya (via GitHub)" <gi...@apache.org> on 2026/07/08 00:46:05 UTC

[PR] [SPARK-58024][PYTHON] Convert Arrow struct and map columns to Python rows in bulk [spark]

viirya opened a new pull request, #57105:
URL: https://github.com/apache/spark/pull/57105

   ### What changes were proposed in this pull request?
   
   Follow-up of SPARK-58019 (#57099) and SPARK-58023 (#57104); **only the last commit is new — this PR is stacked on #57104 and will be rebased once its predecessors merge.**
   
   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.
   
   ### 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>` | 914 ms | 175 ms | 5.2x |
   | `map<string,int64>` (2 entries/row) | 2.07 s | 303 ms | 6.8x |
   
   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).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58024][PYTHON] Convert Arrow struct and map columns to Python rows in bulk [spark]

Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on code in PR #57105:
URL: https://github.com/apache/spark/pull/57105#discussion_r3562719812


##########
python/pyspark/sql/conversion.py:
##########
@@ -1043,9 +1046,44 @@ def _to_pylist(column: Union["pa.Array", "pa.ChunkedArray"]) -> List[Any]:
                 result.extend(ArrowTableToRowsConversion._to_pylist(chunk))
             return result
 
-        if (pa.types.is_list(column.type) or pa.types.is_large_list(column.type)) and len(
-            column
-        ) > 0:
+        if len(column) == 0:
+            return []
+
+        if pa.types.is_map(column.type):
+            # Maps have the same offsets layout as lists; each row becomes a
+            # list of (key, value) tuples, matching MapScalar.as_py.
+            n = len(column)
+            offsets = column.offsets.to_numpy(zero_copy_only=True).tolist()
+            start = offsets[0]
+            length = offsets[-1] - start
+            keys = ArrowTableToRowsConversion._to_pylist(column.keys.slice(start, length))
+            items = ArrowTableToRowsConversion._to_pylist(column.items.slice(start, length))
+            if column.null_count == 0:
+                return [
+                    list(
+                        zip(
+                            keys[offsets[i] - start : offsets[i + 1] - start],
+                            items[offsets[i] - start : offsets[i + 1] - start],
+                        )
+                    )
+                    for i in range(n)
+                ]
+            valid = column.is_valid().to_numpy(zero_copy_only=False).tolist()
+            return [
+                (
+                    list(
+                        zip(
+                            keys[offsets[i] - start : offsets[i + 1] - start],
+                            items[offsets[i] - start : offsets[i + 1] - start],
+                        )
+                    )
+                    if valid[i]
+                    else None
+                )
+                for i in range(n)
+            ]
+
+        if pa.types.is_list(column.type) or pa.types.is_large_list(column.type):

Review Comment:
   Done in fc45dd79539 — both changed to `elif`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58024][PYTHON] Convert Arrow struct and map columns to Python rows in bulk [spark]

Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya closed pull request #57105: [SPARK-58024][PYTHON] Convert Arrow struct and map columns to Python rows in bulk
URL: https://github.com/apache/spark/pull/57105


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58024][PYTHON] Convert Arrow struct and map columns to Python rows in bulk [spark]

Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on PR #57105:
URL: https://github.com/apache/spark/pull/57105#issuecomment-4942762795

   Thanks @gaogaotiantian 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58024][PYTHON] Convert Arrow struct and map columns to Python rows in bulk [spark]

Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on PR #57105:
URL: https://github.com/apache/spark/pull/57105#issuecomment-4940341422

   @huaxingao Can you hold 4.2 new RC cut until this gets merged? This is to extend #57099 to struct and map columns. Thanks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58024][PYTHON] Convert Arrow struct and map columns to Python rows in bulk [spark]

Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on PR #57105:
URL: https://github.com/apache/spark/pull/57105#issuecomment-4942759756

   **Merge Summary:**
   - merged into master https://github.com/apache/spark/commit/9c6e57bf7e0c6f5ab4870d28611449d36ab7c99c
   - merged into branch-4.x https://github.com/apache/spark/commit/59740918f988946ea07e728d6e1e46d27f1a5bbe
   - merged into branch-4.2 https://github.com/apache/spark/commit/44daf0131f7bd8be5e4c6864e19d73b0dd078dca
   
   *Posted by `merge_spark_pr.py`*


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58024][PYTHON] Convert Arrow struct and map columns to Python rows in bulk [spark]

Posted by "gaogaotiantian (via GitHub)" <gi...@apache.org>.
gaogaotiantian commented on code in PR #57105:
URL: https://github.com/apache/spark/pull/57105#discussion_r3562684695


##########
python/pyspark/sql/conversion.py:
##########
@@ -1043,9 +1046,44 @@ def _to_pylist(column: Union["pa.Array", "pa.ChunkedArray"]) -> List[Any]:
                 result.extend(ArrowTableToRowsConversion._to_pylist(chunk))
             return result
 
-        if (pa.types.is_list(column.type) or pa.types.is_large_list(column.type)) and len(
-            column
-        ) > 0:
+        if len(column) == 0:
+            return []
+
+        if pa.types.is_map(column.type):
+            # Maps have the same offsets layout as lists; each row becomes a
+            # list of (key, value) tuples, matching MapScalar.as_py.
+            n = len(column)
+            offsets = column.offsets.to_numpy(zero_copy_only=True).tolist()
+            start = offsets[0]
+            length = offsets[-1] - start
+            keys = ArrowTableToRowsConversion._to_pylist(column.keys.slice(start, length))
+            items = ArrowTableToRowsConversion._to_pylist(column.items.slice(start, length))
+            if column.null_count == 0:
+                return [
+                    list(
+                        zip(
+                            keys[offsets[i] - start : offsets[i + 1] - start],
+                            items[offsets[i] - start : offsets[i + 1] - start],
+                        )
+                    )
+                    for i in range(n)
+                ]
+            valid = column.is_valid().to_numpy(zero_copy_only=False).tolist()
+            return [
+                (
+                    list(
+                        zip(
+                            keys[offsets[i] - start : offsets[i + 1] - start],
+                            items[offsets[i] - start : offsets[i + 1] - start],
+                        )
+                    )
+                    if valid[i]
+                    else None
+                )
+                for i in range(n)
+            ]
+
+        if pa.types.is_list(column.type) or pa.types.is_large_list(column.type):

Review Comment:
   nit: could you change this `if` to `elif`? otherwise looks good to me.



##########
python/pyspark/sql/conversion.py:
##########
@@ -1043,9 +1046,44 @@ def _to_pylist(column: Union["pa.Array", "pa.ChunkedArray"]) -> List[Any]:
                 result.extend(ArrowTableToRowsConversion._to_pylist(chunk))
             return result
 
-        if (pa.types.is_list(column.type) or pa.types.is_large_list(column.type)) and len(
-            column
-        ) > 0:
+        if len(column) == 0:
+            return []
+
+        if pa.types.is_map(column.type):
+            # Maps have the same offsets layout as lists; each row becomes a
+            # list of (key, value) tuples, matching MapScalar.as_py.
+            n = len(column)
+            offsets = column.offsets.to_numpy(zero_copy_only=True).tolist()
+            start = offsets[0]
+            length = offsets[-1] - start
+            keys = ArrowTableToRowsConversion._to_pylist(column.keys.slice(start, length))
+            items = ArrowTableToRowsConversion._to_pylist(column.items.slice(start, length))
+            if column.null_count == 0:
+                return [
+                    list(
+                        zip(
+                            keys[offsets[i] - start : offsets[i + 1] - start],
+                            items[offsets[i] - start : offsets[i + 1] - start],
+                        )
+                    )
+                    for i in range(n)
+                ]
+            valid = column.is_valid().to_numpy(zero_copy_only=False).tolist()
+            return [
+                (
+                    list(
+                        zip(
+                            keys[offsets[i] - start : offsets[i + 1] - start],
+                            items[offsets[i] - start : offsets[i + 1] - start],
+                        )
+                    )
+                    if valid[i]
+                    else None
+                )
+                for i in range(n)
+            ]
+
+        if pa.types.is_list(column.type) or pa.types.is_large_list(column.type):

Review Comment:
   nit: could you change this `if` and the one below to `elif`? otherwise looks good to me.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58024][PYTHON] Convert Arrow struct and map columns to Python rows in bulk [spark]

Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on PR #57105:
URL: https://github.com/apache/spark/pull/57105#issuecomment-4940347196

   @dongjoon-hyun @Yicong-Huang  @gaogaotiantian This extends the optimization to struct and map columns. Could you also take a look? Thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58024][PYTHON] Convert Arrow struct and map columns to Python rows in bulk [spark]

Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on PR #57105:
URL: https://github.com/apache/spark/pull/57105#issuecomment-4942787568

   @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 is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org