kosiew commented on code in PR #24924:
URL: https://github.com/apache/datafusion/pull/24924#discussion_r3956784747
##########
datafusion/common/src/config.rs:
##########
@@ -1046,6 +1046,12 @@ config_namespace! {
/// number of rows written is not roughly divisible by the soft max
pub soft_max_rows_per_output_file: ConfigNonZeroUsize, default =
non_zero_usize_default(50000000)
+ /// Target number of bytes in output files when writing multiple.
Review Comment:
Could we also add a `SET datafusion.execution.soft_max_bytes_per_output_file
= 0` error case in `set_variable.slt`? `ConfigNonZeroUsize` already rejects
zero, but adding coverage through the public SQL configuration path would match
the existing row-limit coverage.
##########
datafusion/datasource/src/write/demux.rs:
##########
@@ -224,6 +237,7 @@ async fn row_count_demuxer(
part_idx += 1;
}
row_counts[next_send_steam] += rb.num_rows();
+ bytes_counts[next_send_steam] += rb.get_array_memory_size();
Review Comment:
I think this needs to measure serialized output bytes rather than the Arrow
allocation size. `get_array_memory_size()` gives us the pre-serialization
RecordBatch size, but Parquet defaults to `zstd(3)`, so a highly compressible
blob batch could be hundreds of MiB in memory and only a small fraction of that
on disk.
In that case, the demuxer would rotate before the next batch and could
create much smaller files than the configured `soft_max_bytes_per_output_file`
suggests. That also seems particularly relevant to the blob use case this
option is intended to help with.
Could we move the accounting to a point where serialized or emitted bytes
are known, and feed that back to the demuxer if needed? If the intent is
instead to use input memory size as an estimate, I think the option and docs
should be narrowed to make that explicit.
It would also be useful to add a regression test using a large, highly
compressible binary column with Parquet compression enabled. The current
parameterized test uses `UNCOMPRESSED`, so it would not catch this difference.
--
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]
