[aetosdios27]

Which issue does this PR close?

Closes #24868.

Rationale for this change

Microbenchmarks can compile successfully but panic when executed. Running their cases in CI catches stale benchmark fixtures and invocation assumptions without collecting performance measurements.

What changes are included in this PR?

• Add cargo xtask ci step test microbenchmarks and a required extended CI job. Following the existing workflow convention, it runs in the merge queue, on release-branch pushes, and manually; it skips PR events.
• Execute Criterion bench targets in test mode with --no-fail-fast, including the physical-plan benchmarks gated by test_utils. The ci-bench profile retains workspace debug assertions and overflow checks with light optimization to keep larger cases practical.
• Use the checked-in ten-row ClickBench fixture for SQL planner smoke tests through CLICKBENCH_DATA_PATH. Ordinary performance runs retain their full-dataset default. The separate SQL benchmark harness, which requires generated datasets, remains covered by its existing jobs.
• Fix benchmark failures exposed by execution: timestamp timezone metadata, Spark slice input coercion, string return types, and repeat row counts/error assertions. Slice coercion happens before timing the UDF.
• Clarify the Spark ListView benchmark labels and check the repeat overflow diagnostic outside the timed loop. Patch two certificate dependencies to resolve the security audit failure.

What is the testing strategy for this PR?

• The full cargo xtask ci step test microbenchmarks run passed all 131 benchmark targets on upstream 99193cbd0. After rebasing onto 6c1727fcd, the Spark slice and repeat benchmark smoke tests passed again, including the new labels and overflow diagnostic assertion.
• cargo fmt --all and cargo clippy --all-targets --all-features -- -D warnings pass with the pinned Rust 1.99 toolchain on the final rebase.
• ./ci/scripts/security_audit.sh passes after patching rustls-webpki to 0.103.16 and webpki-root-certs to 1.0.10.
• Extended workspace tests were run with the documented features. The run stopped at the existing sort_with_mem_limit_1_runner RSS assertion: the core integration target had 1,463 passing tests, one failure, and one ignored test. Its RSS monitor reported a zero-byte baseline and a 192.2 MB peak against a 190.7 MB limit. The unchanged test passed in isolation on a clean upstream build, but the full run still fails locally. No tests were skipped and no memory-test code or limits were changed.

Are there any user-facing changes?

No query behavior or public API changes. Benchmark contributors gain a local smoke-test command and automated execution coverage.

[kosiew]

@aetosdios27
I would love to review this after the merge conflicts are resolved.

[aetosdios27]

@kosiew Conflicts are resolved: rebased onto current main. The power and left_right bench fixes were already covered upstream, so I dropped them from this PR. The full cargo xtask ci step test microbenchmarks run passes locally. Ready for review, thanks!

[kosiew]

@aetosdios27,

Thanks for adding microbenchmark execution to extended CI. The approach looks good and addresses runtime failures that compilation alone cannot catch.

The boundary is appropriate: benchmark correctness is checked through Criterion test mode, separately from performance measurement. The benchmark fixes update stale invocation contracts at setup rather than masking failures, without changing production SQL or public APIs. The modest additional CI complexity is justified by the centralized command and explicit fixture selection.

I'm happy to approve. I left two optional suggestions to clarify the Spark slice benchmark labels and strengthen the repeat overflow smoke check. Neither is blocking.

[kosiew]

Could we label the ListView cases as ListView -> List (coercion excluded)? Coercion happens before timing, so these measure execution on List rather than native ListView or conversion cost. Including both types would distinguish them from existing List cases.

[kosiew]

Optional: could we check once outside the timed loop that the overflow error contains string size overflow on repeat? expect_err also accepts unrelated errors. The production regression test already checks this diagnostic, so this is only a smoke-check improvement.

[aetosdios27]

@kosiew Addressed both suggestions: the slice cases now say ListView -> List (coercion excluded), and repeat checks the overflow diagnostic once outside the timed loop. Both benchmark smoke tests pass. Thanks for the review!

[github-actions]

Thank you for opening this pull request!

Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).

Details

     Cloning apache/main
    Building datafusion v55.1.0 (current)
       Built [  48.172s] (current)
     Parsing datafusion v55.1.0 (current)
      Parsed [   0.029s] (current)
    Building datafusion v55.1.0 (baseline)
       Built [  45.295s] (baseline)
     Parsing datafusion v55.1.0 (baseline)
      Parsed [   0.031s] (baseline)
    Checking datafusion v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.793s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  96.122s] datafusion
    Building datafusion-functions v55.1.0 (current)
       Built [  26.639s] (current)
     Parsing datafusion-functions v55.1.0 (current)
      Parsed [   0.085s] (current)
    Building datafusion-functions v55.1.0 (baseline)
       Built [  26.842s] (baseline)
     Parsing datafusion-functions v55.1.0 (baseline)
      Parsed [   0.085s] (baseline)
    Checking datafusion-functions v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.493s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  55.378s] datafusion-functions
    Building datafusion-spark v55.1.0 (current)
       Built [  37.322s] (current)
     Parsing datafusion-spark v55.1.0 (current)
      Parsed [   0.059s] (current)
    Building datafusion-spark v55.1.0 (baseline)
       Built [  37.594s] (baseline)
     Parsing datafusion-spark v55.1.0 (baseline)
      Parsed [   0.061s] (baseline)
    Checking datafusion-spark v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.480s] 229 checks: 226 pass, 3 fail, 0 warn, 31 skip

--- failure function_missing: pub fn removed or renamed ---

Description:
A publicly-visible function cannot be imported by its prior path. A `pub use` may have been removed, or the function itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/function_missing.ron

Failed in:
  function datafusion_spark::function::generator::stack, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/17be608764acea4466f5890c2757b7f698f016fa/datafusion/spark/src/function/generator/mod.rs:24
  function datafusion_spark::function::generator::expr_fn::stack, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/17be608764acea4466f5890c2757b7f698f016fa/datafusion/spark/src/function/generator/mod.rs:29
  function datafusion_spark::expr_fn::stack, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/17be608764acea4466f5890c2757b7f698f016fa/datafusion/spark/src/function/generator/mod.rs:29

--- failure module_missing: pub module removed or renamed ---

Description:
A publicly-visible module cannot be imported by its prior path. A `pub use` may have been removed, or the module may have been renamed, removed, or made non-public.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/module_missing.ron

Failed in:
  mod datafusion_spark::function::generator::stack, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/17be608764acea4466f5890c2757b7f698f016fa/datafusion/spark/src/function/generator/stack.rs:18

--- failure struct_missing: pub struct removed or renamed ---

Description:
A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/struct_missing.ron

Failed in:
  struct datafusion_spark::function::generator::stack::SparkStack, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/17be608764acea4466f5890c2757b7f698f016fa/datafusion/spark/src/function/generator/stack.rs:62

     Summary semver requires new major version: 3 major and 0 minor checks failed
    Finished [  76.730s] datafusion-spark

[kosiew]

@aetosdios27,

Thanks for adding microbenchmark execution to extended CI and addressing the follow-up feedback. The approach is appropriate, and both optional review requests have been implemented and independently smoke-tested.

Keeping benchmark correctness checks separate from performance measurement is the right architectural boundary. It catches runtime failures and stale invocation assumptions without introducing performance measurements into CI or changing production behavior.

The follow-up changes correctly clarify that Spark ListView coercion is excluded from timing and validate the expected repeat overflow error outside the timed loop. I found no new defects or remaining actionable concerns, so I'm approving. CI checks are still pending and should complete before merging.

[kosiew]

🚀
@aetosdios27
Thank you for your contribution.

[alamb]

This job takes 10 minutes of runner time to run. Do we really need to add that much extra CI time?

Here is an example:

• https://github.com/apache/datafusion/actions/runs/39797768678/job/119889489476

[kosiew]

@alamb

This job takes 10 minutes of runner time to run

Whoops!

Shall I raise a PR to amend this to manual execution only?

• Remove the full microbenchmark job from merge-queue execution and required checks; retain an explicit manual invocation.
• Ask authors to execute affected benchmarks when changing fixtures or benchmark APIs, and record the command/results in their PR.

[alamb]

Shall I raise a PR to amend this to manual execution only?

Thank you @kosiew 🙏

That would be my preference -- or maybe just remove the CI check entirely (leave the xtask target) and add some agent instructions 🤔