Copilot commented on code in PR #10148:
URL: https://github.com/apache/paimon/pull/10148#discussion_r4082677425
##########
paimon-python/pypaimon/data/map_shared_shredding.py:
##########
@@ -240,7 +240,19 @@ def assemble_shared_shredding_map(
pa.types.is_list(mapping_column.type)
or pa.types.is_large_list(mapping_column.type)):
raise TypeError("Shared-shredding field mapping must be an array")
- mapping = mapping_column.to_pylist()
+ mapping_values = mapping_column.values
+ # Convert integer mapping buffers in bulk instead of boxing each Arrow
scalar.
+ # Tiny batches do not amortize the offsets and list reconstruction
overhead.
+ if len(column) >= 32 and num_columns > 0 and not mapping_values.null_count:
Review Comment:
`not mapping_values.null_count` is a readability footgun and can behave
unexpectedly if `null_count` is ever `-1` (unknown). Prefer an explicit
comparison (`mapping_values.null_count == 0`) to make the intended guard
unambiguous.
##########
paimon-python/pypaimon/data/map_shared_shredding.py:
##########
@@ -240,7 +240,19 @@ def assemble_shared_shredding_map(
pa.types.is_list(mapping_column.type)
or pa.types.is_large_list(mapping_column.type)):
raise TypeError("Shared-shredding field mapping must be an array")
- mapping = mapping_column.to_pylist()
+ mapping_values = mapping_column.values
+ # Convert integer mapping buffers in bulk instead of boxing each Arrow
scalar.
+ # Tiny batches do not amortize the offsets and list reconstruction
overhead.
+ if len(column) >= 32 and num_columns > 0 and not mapping_values.null_count:
+ mapping_offsets, mapping_start, mapping_end =
_normalized_offsets(mapping_column)
+ flat_mapping = mapping_values.slice(mapping_start, mapping_end -
mapping_start).to_numpy().tolist()
+ mapping = [
+ None if is_null else flat_mapping[start:end]
+ for start, end, is_null in zip(mapping_offsets,
mapping_offsets[1:], mapping_column.is_null().to_pylist())
Review Comment:
In the fast path, `mapping_column.is_null().to_pylist()` allocates and
converts a full boolean array even when there are no null lists. If
`mapping_column.null_count == 0`, you can skip computing `is_null()` entirely
(e.g., treat all rows as non-null) to reduce overhead in the optimized path.
--
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]
