andygrove commented on code in PR #7234:
URL: https://github.com/apache/datafusion-comet/pull/7234#discussion_r4414893753
##########
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:
Done. The shuffle_writer tests now call the shared `decode_blocks`, and the
local `read_all_ipc_batches` copy is gone (17829e9d6d).
##########
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:
Agreed. The reasoning now lives in two places: the `PartitionWriter` trait
docs and `BufBatchWriter::new_for_partition`. I removed the repeat on
`finish_partition`, `PartitionedSpill::write`, the `DataOutput` docs and the
`finish_partition` comment in `local_partition_writer.rs`, and the
`BufBatchWriter` type-level bullet just names the mode. The other sites now
describe only the call (17829e9d6d).
--
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]
