You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "comphead (via GitHub)" <gi...@apache.org> on 2026/10/24 05:02:38 UTC

[I] Run the sqllogictest corpus across a matrix of batch sizes [datafusion]

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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Run the sqllogictest corpus across a matrix of batch sizes [datafusion]

Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on issue #26700:
URL: https://github.com/apache/datafusion/issues/26700#issuecomment-6380271716

   >     The current approach applies one batch size matrix per file, but I think configuring batch sizes **per query** would be more flexible.
   
   I feel we can generalize it next way:
   - correctness (response should be the same no matter what batchsize set)
   - auxiliary like EXPLAIN, etc


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Run the sqllogictest corpus across a matrix of batch sizes [datafusion]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #26700:
URL: https://github.com/apache/datafusion/issues/26700#issuecomment-6380205406

   I'd prefer to test multiple batch sizes only when the batch size actually matters. Otherwise, we can just test with the default size to minimize the number of tests we run while keeping the same coverage.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Run the sqllogictest corpus across a matrix of batch sizes [datafusion]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #26700:
URL: https://github.com/apache/datafusion/issues/26700#issuecomment-6380252907

   > 3\. I'd prefer to test multiple batch sizes only when the batch size actually matters. Otherwise, we can just test with the default size to minimize the number of tests we run while keeping the same coverage.
   
   The current approach applies one batch size matrix per file, but I think configuring batch sizes **per query** would be more flexible.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Run the sqllogictest corpus across a matrix of batch sizes [datafusion]

Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on issue #26700:
URL: https://github.com/apache/datafusion/issues/26700#issuecomment-6456273292

   Thanks @2010YOUY01 @jayzhan211 I'll come with first PR soon and we can discuss there


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Run the sqllogictest corpus across a matrix of batch sizes [datafusion]

Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on issue #26700:
URL: https://github.com/apache/datafusion/issues/26700#issuecomment-6380240125

   These are the `slt` files to amend, grouped by operator. Paths are relative to `datafusion/sqllogictest/test_files/`.
   
   Markers:
   - **E**: the file contains `EXPLAIN`, so it needs a plan/matrix split (or the runner has to skip `EXPLAIN` for non-default sizes).
   - **S**: the file sets `datafusion.execution.batch_size` itself, so that `SET` has to be removed or reconciled with the sweep.
   
   ### Aggregation
   - [ ] `aggregate.slt` (E)
   - [ ] `aggregate_any_value.slt`
   - [ ] `aggregate_filter_selection.slt` (S)
   - [ ] `aggregate_memory_spill.slt` (E, S)
   - [ ] `aggregate_repartition.slt` (E)
   - [ ] `aggregate_skip_partial.slt`
   - [ ] `aggregates_topk.slt` (E)
   - [ ] `array_agg.slt` (E)
   - [ ] `distinct_on.slt` (E)
   - [ ] `first_last_nested.slt`
   - [ ] `first_last_ordered.slt`
   - [ ] `group.slt`
   - [ ] `group_by.slt` (E)
   - [ ] `grouping.slt` (E)
   - [ ] `grouping_set_repartition.slt` (E)
   - [ ] `ordered_aggregate_key_types.slt` (E, S)
   - [ ] `ordered_aggregate_spill.slt` (E, S)
   - [ ] `range_sorted_time_bin_agg.slt` (E)
   - [ ] `single_distinct_to_groupby.slt` (E)
   - [ ] `min_max/*.slt`
   - [ ] `spark/aggregate/avg.slt`, `spark/aggregate/collect.slt`, `spark/aggregate/try_sum.slt`
   
   ### Windows
   - [ ] `window.slt` (E, S)
   - [ ] `window_limits.slt` (E)
   - [ ] `window_topk_pushdown.slt` (E)
   - [ ] `window_topn.slt` (E)
   - [ ] `array_agg_sliding_window.slt`
   - [ ] `lead_lag_extreme_offsets.slt`
   - [ ] `qualify.slt` (E)
   - [ ] `pg_compat/pg_compat_window.slt`
   - [ ] `spark/aggregate/collect_window.slt`
   
   ### Joins
   - [ ] `joins.slt` (E)
   - [ ] `join.slt.part` (E), included by `join_only.slt` and `join_disable_repartition_joins.slt` (E)
   - [ ] `join_is_not_distinct_from.slt` (E)
   - [ ] `join_limit_pushdown.slt` (E)
   - [ ] `join_lists.slt`
   - [ ] `join_dynamic_filter_transfer.slt` (E)
   - [ ] `joins_conditionless.slt`
   - [ ] `same_column_name_cross_join.slt`
   - [ ] `asof_join.slt` (E)
   - [ ] `lateral_join.slt` (E)
   - [ ] `sort_merge_join.slt` (E)
   - [ ] `sort_merge_join_batches.slt` (E, S)
   - [ ] `sort_merge_join_spill.slt` (E, S)
   - [ ] `pwmj.slt` (E)
   - [ ] `piecewise_merge_join_batches.slt` (E, S)
   - [ ] `nested_loop_join_existence.slt` (E, S)
   - [ ] `nested_loop_join_lambda.slt` (E)
   - [ ] `nested_loop_join_spill.slt` (E, S)
   - [ ] `null_aware_anti_join.slt` (E, S)
   - [ ] `null_aware_mark_join.slt` (E, S)
   
   These files already sweep `batch_size` with `configMatrix`: `sort_merge_join_matrix.slt`, `piecewise_merge_join_matrix.slt`, `piecewise_merge_join_nested_nulls.slt`, `mark_join_matrix.slt`, `null_aware_multi_column.slt`.
   
   ### Limits, TopK and sort
   - [ ] `limit.slt` (E)
   - [ ] `limit_single_row_batches.slt`
   - [ ] `limit_pruning.slt` (E)
   - [ ] `topk.slt` (E)
   - [ ] `push_down_topk_through_join.slt` (E)
   - [ ] `order.slt` (E)
   - [ ] `order_by_all.slt`
   - [ ] `sort_pushdown.slt` (E)
   - [ ] `subquery_sort.slt` (E)
   
   ### Set operations
   - [ ] `union.slt` (E, S)
   - [ ] `union_by_name.slt`
   - [ ] `intersection.slt`
   - [ ] `pg_compat/pg_compat_union.slt`
   
   ### Unnest and generators
   - [ ] `unnest.slt` (E)
   - [ ] `unnest_struct_nulls.slt`
   - [ ] `push_down_filter_unnest.slt` (E)
   - [ ] `spark/generator/stack.slt`
   
   ### Recursive CTE and subqueries
   - [ ] `cte.slt` (E, S)
   - [ ] `cte_materialized.slt` (E)
   - [ ] `subquery.slt` (E, S)
   - [ ] `subquery_projection.slt` (E)
   
   ### Repartition
   - [ ] `repartition.slt` (E)
   - [ ] `repartition_scan.slt` (E)
   - [ ] `range_partitioning.slt` (E)
   
   ### End-to-end query suites
   - [ ] `tpch/tpch.slt`, which includes `tpch/answers/*.slt.part` (no `EXPLAIN`) and `tpch/plans/*.slt.part` (E)
   - [ ] `clickbench.slt` (E)
   - [ ] `imdb.slt`
   
   I left out files that mainly test scalar functions, casts, DDL/DML, optimizer rewrites or file formats. Their results are not expected to depend on how batches are split, but I have not checked this. With the runner-flag approach from the description, they would be swept anyway, at no extra editing cost.
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Run the sqllogictest corpus across a matrix of batch sizes [datafusion]

Posted by "2010YOUY01 (via GitHub)" <gi...@apache.org>.
2010YOUY01 commented on issue #26700:
URL: https://github.com/apache/datafusion/issues/26700#issuecomment-6382028296

   Thanks for driving this! IMO, this is the highest-ROI improvement we can make to our testing right now.
   
   ### Suggestion: make it more automatic
   
   I think the existing practice is manual fuzzing:
   
   - per query: set different `batch_size` values and repeat the same query in an `slt` file
   - per file: config matrix
   
   I think it would be great if we could make this more automatic (no need to manually define which `batch_size` values to test).
   
   The implementation might look like:
   
   - For common configs like `batch_size`/`target_partitions`, let the SLT runner decide the values (Q1: batch_size=8192, Q2: batch_size=7, ...)
   - For feature-specific flags, keep using per-file/per-query config matrices
   - Add a mechanism to skip EXPLAIN-like tests
   
   Then the runner could generate random configs across different fuzzing runs, and we could set the CI runner to run as many tests as possible within a time limit, instead of enumerating all combinations, which could become very slow as we keep adding config matrix dimensions.
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Run the sqllogictest corpus across a matrix of batch sizes [datafusion]

Posted by "2010YOUY01 (via GitHub)" <gi...@apache.org>.
2010YOUY01 commented on issue #26700:
URL: https://github.com/apache/datafusion/issues/26700#issuecomment-6382039139

   > > ```
   > > The current approach applies one batch size matrix per file, but I think configuring batch sizes **per query** would be more flexible.
   > > ```
   > 
   > I feel we can generalize it next way:
   > 
   > * correctness (response should be the same no matter what batchsize set)
   > * auxiliary like EXPLAIN, etc
   
   If we do some refactor to split existing correctness and EXPLAIN tests, config fuzzing will likely become easier 🤔 


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Run the sqllogictest corpus across a matrix of batch sizes [datafusion]

Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on issue #26700:
URL: https://github.com/apache/datafusion/issues/26700#issuecomment-6380256700

   >  I'd prefer to test multiple batch sizes only when the batch size actually matters. Otherwise, we can just test with the default size to minimize the number of tests we run while keeping the same coverage.
   
   thats my vision too, start with operators. joins/windows/aggregations/topk/limit, etc


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org