comphead commented on code in PR #7234:
URL: https://github.com/apache/datafusion-comet/pull/7234#discussion_r4407318953
##########
native/shuffle/src/writers/partition_writer.rs:
##########
@@ -54,6 +61,9 @@ pub(crate) trait PartitionWriter: Send {
///
/// Must be called exactly once per partition, in ascending id order, so
the
/// writer can lay partitions out contiguously and record their offsets.
+ ///
+ /// `iter` should yield maximal `batch_size` chunks plus at most one
smaller tail, as
+ /// `PartitionedBatchIterator` does.
Review Comment:
This repeats the trait-level note added above, which already covers both
`write` and `finish_partition`. The same "maximal chunks plus one tail, so pass
through" reasoning is also restated on `PartitionedSpill::write` in `spill.rs`,
in the `finish_partition` comment in `local_partition_writer.rs`, and on
`BufBatchWriter`. Could the reasoning live once, on
`BufBatchWriter::new_for_partition` and the trait docs, with the other sites
reduced to the call?
##########
native/shuffle/src/writers/mod.rs:
##########
@@ -28,3 +28,16 @@ pub(crate) use
local::local_partition_writer::LocalPartitionWriter;
pub(crate) use partition_writer::PartitionWriter;
pub(crate) use rss::rss_partition_writer::RssPartitionWriter;
pub use shuffle_block_writer::{CompressionCodec, ShuffleBlockWriter};
+
+/// Decodes every length-prefixed shuffle block in `bytes`, in order.
+#[cfg(test)]
+pub(crate) fn decode_blocks(bytes: &[u8]) -> Vec<arrow::array::RecordBatch> {
Review Comment:
Now that this is a crate-level test helper, `shuffle_writer.rs` has the same
loop as `read_all_ipc_batches` (length prefix, skip the 16-byte header,
`read_ipc_compressed`), used by `read_all_ipc_blocks` and the tests at its
other call sites. Could those call `decode_blocks` and the copy in
`shuffle_writer.rs` go away?
--
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]
