DevShiba opened a new pull request, #24204:
URL: https://github.com/apache/datafusion/pull/24204

   ## Which issue does this PR close?
   
   - Part of https://github.com/apache/datafusion/issues/17498
   
   ## Rationale for this change
   
   ```
   DataFusion CLI v54.1.0
   > set datafusion.execution.max_buffered_batches_per_output_file = 1;
   > COPY (SELECT 1 as a) TO '/tmp/x.parquet';
   
   thread 'tokio-rt-worker' panicked at 
datafusion/datasource/src/write/demux.rs:287:30:
   mpsc bounded channel requires buffer > 0
   ```
   
   Two call sites (`demux.rs::create_new_file_stream`, 
`orchestration.rs::spawn_writer_tasks_and_join`) divide 
`max_buffered_batches_per_output_file` in half to size a bounded `mpsc` 
channel's capacity. Integer division rounds both 0 *and* 1 down to 0, and 
Tokio's `mpsc::channel` panics on a zero capacity. A plain non-zero check (like 
the existing `ConfigNonZeroUsize`) would not have been sufficient here, since 1 
also triggers the panic.
   
   A third call site (`demux.rs::hive_style_partitions_demuxer`) uses the raw 
value directly without dividing, so it would panic on 0 alone.
   
   ## What changes are included in this PR?
   
   Adds `ConfigMinTwoUsize`, mirroring the existing `ConfigNonZeroUsize` 
pattern already used for sibling fields (`batch_size`, 
`meta_fetch_concurrency`, `minimum_parallel_output_files`, etc.), and applies 
it to `max_buffered_batches_per_output_file`. Invalid values are now rejected 
with a clear configuration error at set-time instead of panicking later at 
write time. Updated the three read sites to call `.get()`, updated the field 
doc comment to explain the constraint, and regenerated 
`docs/source/user-guide/configs.md` via `dev/update_config_docs.sh`.
   
   ## Are these changes tested?
   
   Yes. Added two `statement error` cases to 
`datafusion/sqllogictest/test_files/set_variable.slt` (values 0 and 1), 
following the exact pattern already used for the sibling `ConfigNonZeroUsize` 
fields in that file. Verified manually with `datafusion-cli` that 0 and 1 now 
return a clean error instead of panicking, and that 2 (the default) and 3 still 
work correctly. Ran `cargo test -p datafusion-datasource --lib` (178 passed), 
the `set_variable.slt` sqllogictest suite, and `cargo check --workspace 
--all-targets` — all clean.
   
   ## Are there any user-facing changes?
   
   Yes: setting `datafusion.execution.max_buffered_batches_per_output_file` to 
`0` or `1` now returns a configuration error instead of panicking. No change 
for any value `>= 2` (including the default of 2).


-- 
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]

Reply via email to