Skip to content

Refresh statistics parser settings between query files - #24738

Draft
EltonChang1 wants to merge 1 commit into
apache:mainfrom
EltonChang1:codex/refresh-statistics-parser-settings
Draft

Refresh statistics parser settings between query files#24738
EltonChang1 wants to merge 1 commit into
apache:mainfrom
EltonChang1:codex/refresh-statistics-parser-settings

Conversation

@EltonChang1

Copy link
Copy Markdown

Which issue does this PR close?

Rationale for this change

The statistics runner previously captured SQL parser settings only once. A successful SET in one query file therefore changed the session but did not affect parsing of later files, which made ordered query suites behave inconsistently with session state.

What changes are included in this PR?

  • Read the current session's parser options immediately before parsing each query file.
  • Extract file processing into a focused helper without changing the existing same-file pre-parsing behavior.
  • Document that parser-setting changes still do not affect later statements in the same already-parsed file.
  • Add ordered-file regressions for MySQL dialect propagation and parser recursion-limit propagation.

Are these changes tested?

  • cargo test -p datafusion-benchmarks --lib statistics::tests -- --nocapture (7 passed)
  • cargo test -p datafusion-benchmarks --lib 'statistics::tests::refreshes_' -- --nocapture after rebasing (2 passed)
  • cargo check -p datafusion-benchmarks --lib
  • cargo clippy -p datafusion-benchmarks --lib -- -D warnings
  • cargo fmt --all -- --check
  • Ablation: both new regressions fail when parsing uses stale default settings.

The broader cargo clippy -p datafusion-benchmarks --all-targets --all-features -- -D warnings could not complete locally because the optional snmalloc target requires cmake, which is unavailable in this environment. It produced no Rust diagnostics before that build-tool failure.

Are there any user-facing changes?

Yes. Parser settings changed by an earlier statistics query file now apply when later query files are parsed. There are no public API changes.

This draft was prepared with AI assistance and has not yet received human review. I understand the implementation end-to-end; there are no known design assumptions beyond the documented same-file parsing limitation.

Signed-off-by: Elton Chang <tchang52@ucsc.edu>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.81481% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.45%. Comparing base (ee59f62) to head (6d9f67b).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
benchmarks/src/statistics.rs 89.81% 9 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24738      +/-   ##
==========================================
+ Coverage   81.42%   81.45%   +0.02%     
==========================================
  Files        1121     1121              
  Lines      402142   402211      +69     
  Branches   402142   402211      +69     
==========================================
+ Hits       327460   327633     +173     
+ Misses      55484    55373     -111     
- Partials    19198    19205       +7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refresh SQL parser settings between statistics query files

2 participants