viirya commented on code in PR #57099:
URL: https://github.com/apache/spark/pull/57099#discussion_r3540595159


##########
python/pyspark/sql/conversion.py:
##########
@@ -980,6 +980,60 @@ class ArrowTableToRowsConversion:
     Conversion from Arrow Table to Rows.
     """
 
+    @staticmethod
+    def _to_pylist(column: Union["pa.Array", "pa.ChunkedArray"]) -> List[Any]:
+        """
+        Equivalent to ``column.to_pylist()``, but converts (nested) list 
columns in bulk
+        instead of one scalar at a time.
+
+        ``Array.to_pylist()`` materializes one Scalar per element; for list 
types each row
+        additionally allocates a C++ scalar, a Python Scalar wrapper and a 
Python Array
+        wrapper for the row's values before converting elements one by one, 
which is
+        several times slower than converting the flattened child values in a 
single pass
+        and slicing the resulting Python list per row (see 
apache/arrow#50326). The values
+        themselves 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 would coerce them to floats/NaN. 
NumPy is used
+        only for the offsets (non-null integers) and the validity bitmap 
(booleans), so no
+        value coercion can occur.
+
+        This can be removed once the minimum supported PyArrow version 
includes the fix
+        for apache/arrow#50326.
+        """
+        import pyarrow as pa
+        import pyarrow.types as pa_types
+
+        try:
+            import numpy  # noqa: F401
+        except ImportError:
+            return column.to_pylist()
+
+        if isinstance(column, pa.ChunkedArray):
+            result: List[Any] = []
+            for chunk in column.chunks:
+                result.extend(ArrowTableToRowsConversion._to_pylist(chunk))
+            return result
+
+        column_type = column.type
+        if (pa_types.is_list(column_type) or 
pa_types.is_large_list(column_type)) and len(
+            column
+        ) > 0:
+            n = len(column)
+            offsets = column.offsets.to_numpy(zero_copy_only=True).tolist()

Review Comment:
   Good point — added the comment in eebbc4fd059. `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.



##########
python/pyspark/sql/conversion.py:
##########
@@ -980,6 +980,60 @@ class ArrowTableToRowsConversion:
     Conversion from Arrow Table to Rows.
     """
 
+    @staticmethod
+    def _to_pylist(column: Union["pa.Array", "pa.ChunkedArray"]) -> List[Any]:
+        """
+        Equivalent to ``column.to_pylist()``, but converts (nested) list 
columns in bulk
+        instead of one scalar at a time.
+
+        ``Array.to_pylist()`` materializes one Scalar per element; for list 
types each row
+        additionally allocates a C++ scalar, a Python Scalar wrapper and a 
Python Array
+        wrapper for the row's values before converting elements one by one, 
which is
+        several times slower than converting the flattened child values in a 
single pass
+        and slicing the resulting Python list per row (see 
apache/arrow#50326). The values
+        themselves 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 would coerce them to floats/NaN. 
NumPy is used
+        only for the offsets (non-null integers) and the validity bitmap 
(booleans), so no
+        value coercion can occur.
+
+        This can be removed once the minimum supported PyArrow version 
includes the fix
+        for apache/arrow#50326.
+        """
+        import pyarrow as pa
+        import pyarrow.types as pa_types
+
+        try:
+            import numpy  # noqa: F401
+        except ImportError:
+            return column.to_pylist()
+
+        if isinstance(column, pa.ChunkedArray):
+            result: List[Any] = []
+            for chunk in column.chunks:
+                result.extend(ArrowTableToRowsConversion._to_pylist(chunk))
+            return result
+
+        column_type = column.type
+        if (pa_types.is_list(column_type) or 
pa_types.is_large_list(column_type)) and len(
+            column
+        ) > 0:
+            n = len(column)
+            offsets = column.offsets.to_numpy(zero_copy_only=True).tolist()
+            start = offsets[0]
+            flat = ArrowTableToRowsConversion._to_pylist(
+                column.values.slice(start, offsets[-1] - start)
+            )
+            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 [

Review 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.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to