comphead opened a new issue, #26700:
URL: https://github.com/apache/datafusion/issues/26700
### Is your feature request related to a problem or challenge?
Almost all `sqllogictest` files run with a single
`datafusion.execution.batch_size` (the default `8192`). Most `slt` tables are
small, so nearly every operator sees one input batch per partition. Code paths
that only run when the input is split across many batches are rarely exercised
end to end: carrying state across batch boundaries, partial emits, coalescing,
buffered join sides, and window frames that span batches.
#20919 showed that this gap hides real bugs. Building with `batch_size = 1`
produced panics, internal assertion failures and wrong results, not just
changed `EXPLAIN` output.
`# configMatrix:` (#24493) can already rerun a file across several values of
a config key. Today only a few files sweep `batch_size` with it
(`sort_merge_join_matrix.slt`, `piecewise_merge_join_matrix.slt`,
`mark_join_matrix.slt`, `null_aware_multi_column.slt`,
`piecewise_merge_join_nested_nulls.slt`). The rest of the corpus, covering
aggregates, windows, sorts, unnest, limits, set operations and the other join
files, runs at one batch size only.
### Describe the solution you'd like
Run the `slt` tests for all operators under a matrix of batch sizes, for
example `1, 2, 7, 8192`. That would be a small size, a size that does not
evenly divide typical row counts, and the default.
A possible shape:
1. Add a runner option, e.g. `--batch-sizes 1,2,7,8192`, that runs every
file once per value. It would reuse the `configMatrix` machinery, so failures
are reported with the combination that produced them. No per-file directive
would be needed.
2. For non-default sizes, check only what does not depend on batch size:
- Skip `EXPLAIN` records, since plans can change with batch size.
- Compare results without regard to row order unless the query has an
`ORDER BY`.
- Give in-file `SET datafusion.execution.batch_size` precedence, or skip
those files for the sweep.
3. Run the sweep in CI, either in the extended-tests job or nightly, so it
does not slow down the main `sqllogictest` run.
4. Fix or file the failures it finds, then make the sweep required.
The default run stays as it is today and still checks plans and exact output.
### Describe alternatives you've considered
- **Add `# configMatrix: datafusion.execution.batch_size=...` to each
file.** This works with the existing runner, but it means editing about 600
`.slt` / `.slt.part` files. About 170 of them contain `EXPLAIN`, so they would
first need to be split into a plan file and a matrix file. It also relies on
every new file opting in.
- **Change the default `batch_size` only in the `slt` test context.** That
tests one other value instead of several, and it changes the expected output of
every `EXPLAIN` that prints the batch size.
- **Fuzz tests per operator.** These are useful, but they cover only the
operators someone writes a fuzzer for and none of the SQL-level shapes in the
corpus.
### Additional context
- #20919: crashes and wrong results with `batch_size = 1`
- #24390 / #24493: `configMatrix` support for `slt`
- #26152: a similar proposal to run the corpus under an alternative
optimizer pipeline using the same machinery
--
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]
