You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/09/08 00:25:09 UTC

(datafusion) branch main updated: fix(benchmarks): validate predicate_eval results (#25037)

This is an automated email from the ASF dual-hosted git repository.

github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git


The following commit(s) were added to refs/heads/main by this push:
     new 8f40f33c21 fix(benchmarks): validate predicate_eval results (#25037)
8f40f33c21 is described below

commit 8f40f33c211937e9b1724925a9ef152f049fccc5
Author: Adrian Garcia Badaracco <17...@users.noreply.github.com>
AuthorDate: Tue Sep 8 00:15:39 2026 +0000

    fix(benchmarks): validate predicate_eval results (#25037)
    
    ## Which issue does this PR close?
    
    N/A - no issue; this is a standalone fix to the benchmark harness.
    
    ## 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)
    
    Co-authored-by: Claude Opus 5 <no...@anthropic.com>
---
 .../sql_benchmarks/predicate_eval/predicate_eval.benchmark.template     | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/benchmarks/sql_benchmarks/predicate_eval/predicate_eval.benchmark.template b/benchmarks/sql_benchmarks/predicate_eval/predicate_eval.benchmark.template
index 0030a7e946..e2e3d6175e 100644
--- a/benchmarks/sql_benchmarks/predicate_eval/predicate_eval.benchmark.template
+++ b/benchmarks/sql_benchmarks/predicate_eval/predicate_eval.benchmark.template
@@ -31,4 +31,6 @@ true
 
 run sql_benchmarks/predicate_eval/queries/${SUBGROUP}/q${QPAD}.sql
 
+result sql_benchmarks/predicate_eval/results/${NAME}.csv
+
 cleanup sql_benchmarks/predicate_eval/init/cleanup.sql


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