sunchao commented on code in PR #5526:
URL: https://github.com/apache/datafusion-comet/pull/5526#discussion_r3924279259
##########
native/shuffle/src/spark_unsafe/row.rs:
##########
@@ -1020,6 +1027,13 @@ fn append_struct_fields_field_major(
}
}
}
+ // A Null field carries no data: every row is null, whether or not
the struct is.
+ DataType::Null => {
+ let field_builder = get_field_builder!(struct_builder,
NullBuilder, field_idx);
+ for _ in row_start..row_end {
Review Comment:
[P2] Reset NullType builders before reusing them for another batch
These new arms handle the first batch, but `process_sorted_row_partition`
creates its builders outside the batch loop and `builder_to_array` calls
`finish()` without replacing them. In the pinned Arrow 58.4.0,
`NullBuilder::finish()` leaves its length unchanged. With the exact current
conversion functions, two consecutive two-row batches of
`struct<v:bigint,n:void>` panic on the second finish because the struct has
length 2 while its Null child has length 4. The single-batch and typed-null
two-batch controls pass. A nested struct fails the same way.
`SpillSorter` sends an entire destination partition to this native call, so
one call can exceed `spark.comet.shuffle.jvm.batchSize`. The PR now admits
producers such as `element_at(transform(array(id), x -> named_struct('v', x,
'n', NULL)), 1)` over native primitive input. With CometShuffleManager, JVM
shuffle mode, `spark.comet.shuffle.convertFromSparkPlan.enabled=false`, and AQE
off, source tracing shows their exchange can enter this writer where the base
retained Spark fallback.
Could we reset or recreate Null-containing builders between batches, or
retain fallback, and add coverage spanning more than one writer batch? I
reproduced the builder failure in an isolated harness using production
conversion source and pinned dependencies. The SQL/planner route was
source-traced, not executed end to end.
--
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]
