adriangb opened a new pull request, #25037:
URL: https://github.com/apache/datafusion/pull/25037
## Which issue does this PR close?
- Closes #.
## Rationale for this change
The `predicate_eval` SQL benchmark template
(`benchmarks/sql_benchmarks/predicate_eval/predicate_eval.benchmark.template`)
had no `result` directive. Every other suite that supports
`--result-mode validate` has one (e.g.
`benchmarks/sql_benchmarks/clickbench/benchmarks/q00.benchmark`,
`benchmarks/sql_benchmarks/h2o/window_sorted.benchmark.template`), but
`predicate_eval` was missing it entirely. As a result,
`benchmark_runner predicate_eval --result-mode validate` silently verified
nothing: `SqlBenchmark::verify_results` returns early when there are no
`result_queries`, so the run reports success without ever comparing query
output against a persisted baseline.
## What changes are included in this PR?
Add a `result` directive to the `predicate_eval` template, after the `run`
line, pointing at a per-query CSV keyed by the benchmark's `${NAME}`
parameter (each `qNN.benchmark` in the suite sets a unique `NAME`), mirroring
the convention used by the `clickbench` and `h2o` suites:
```
result sql_benchmarks/predicate_eval/results/${NAME}.csv
```
No expected-result CSVs are committed — like the other suites, these are
generated locally with `--result-mode persist` (`sql_benchmarks/*/results/`
is excluded from `.gitignore`'s general `results` rule but no CSVs under it
are tracked in the repo today).
## What is the testing strategy for this PR?
Built `benchmark_runner` in release mode and ran the `predicate_eval`
`costsel` subgroup with `PRED_ROWS=100000` to keep it fast:
1. `--result-mode persist` — writes one CSV per query under
`sql_benchmarks/predicate_eval/results/`, e.g.
`costsel_q01_regexp_selective_last.csv` containing `count(*)` / `51`.
2. `--result-mode validate` — passes cleanly against the persisted CSVs.
3. **Negative control**: hand-edited one persisted CSV's value from `51` to
`999999` and re-ran `--result-mode validate`. It now fails loudly:
```
Error: Execution error: Error in result on row 1, column 1 running query
"":
expected value "999999" but got value "51" in row: ["51"]
```
(exit code 1). Restored the file afterward.
4. **Before-the-fix demonstration**: reverted just the template change (via
`git stash`) with no result CSVs present on disk, and reran
`--result-mode validate`. It exited 0 and printed normal benchmark timing
output with no validation performed at all — confirming the bug this PR
fixes.
## Are there any user-facing changes?
No public API changes. This only affects the benchmark harness
(`benchmarks/`), enabling `predicate_eval` to actually validate query
results when run with `--result-mode validate`, matching every other SQL
benchmark suite.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
