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/23 10:15:05 UTC
(datafusion) branch gh-readonly-queue/main/pr-24738-7570366fd929daf9ced744bb8397686b50565b18 created (now 99f733e850)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-24738-7570366fd929daf9ced744bb8397686b50565b18
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at 99f733e850 Refresh statistics parser settings between query files (#24738)
This branch includes the following new commits:
new 99f733e850 Refresh statistics parser settings between query files (#24738)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: Refresh statistics parser settings between query files (#24738)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-24738-7570366fd929daf9ced744bb8397686b50565b18
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 99f733e85094e7a6ea68bc3d90e92fd7d877eb8b
Author: Elton Chang <80...@users.noreply.github.com>
AuthorDate: Wed Sep 23 09:16:07 2026 +0000
Refresh statistics parser settings between query files (#24738)
## Which issue does this PR close?
- Closes #24719.
## 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 <tc...@ucsc.edu>
---
benchmarks/src/statistics.rs | 186 ++++++++++++++++++++++++++++++++-----------
1 file changed, 141 insertions(+), 45 deletions(-)
diff --git a/benchmarks/src/statistics.rs b/benchmarks/src/statistics.rs
index 5c0be9984e..6c14264356 100644
--- a/benchmarks/src/statistics.rs
+++ b/benchmarks/src/statistics.rs
@@ -41,8 +41,10 @@ use serde::{Deserialize, Serialize};
/// Generate reports that compare planning statistics with runtime metrics.
///
-/// Parser options are captured when the run starts, so `SET` statements do not
-/// affect parsing in later query files.
+/// Parser options are refreshed before each query file is parsed, so `SET`
+/// statements affect later files. Because each file is parsed before any of its
+/// statements are executed, parser-setting changes do not affect later
+/// statements in the same file.
#[derive(Debug, Args)]
#[command(verbatim_doc_comment)]
pub struct RunOpt {
@@ -68,7 +70,6 @@ impl RunOpt {
let mut config = SessionConfig::from_env()?.with_collect_statistics(true);
config.options_mut().optimizer.prefer_hash_join = true;
let ctx = SessionContext::new_with_config(config);
- let sql_parser_options = ctx.state().config_options().sql_parser.clone();
register_parquet_files(&ctx, &self.path).await?;
let branch = current_branch_name();
@@ -87,48 +88,8 @@ impl RunOpt {
let mut reports = vec![];
for query_path in query_files(&self.query_path, self.query.as_deref())? {
- let query = query_path
- .file_stem()
- .expect("query file has a filename")
- .to_string_lossy()
- .to_string();
- let sql = fs::read_to_string(query_path)?;
- let statements = match sql_statements(&sql, &sql_parser_options) {
- Ok(statements) => statements,
- Err(error) => {
- let report = QueryReport {
- query: query.clone(),
- statement: 1,
- operators: vec![],
- success: false,
- error: Some(error.to_string()),
- };
- print_query_report(&report, previous.as_deref());
- reports.push(report);
- continue;
- }
- };
- for (statement, sql) in statements.into_iter().enumerate() {
- let statement = statement + 1;
- let report = match self.report_statement(&ctx, sql).await {
- Ok(operators) => QueryReport {
- query: query.clone(),
- statement,
- operators,
- success: true,
- error: None,
- },
- Err(error) => QueryReport {
- query: query.clone(),
- statement,
- operators: vec![],
- success: false,
- error: Some(error.to_string()),
- },
- };
- print_query_report(&report, previous.as_deref());
- reports.push(report);
- }
+ self.report_query_file(&ctx, &query_path, previous.as_deref(), &mut reports)
+ .await?;
}
store_report(&result_path, &reports)?;
print_q_error_summary(
@@ -140,6 +101,59 @@ impl RunOpt {
Ok(())
}
+ async fn report_query_file(
+ &self,
+ ctx: &SessionContext,
+ query_path: &Path,
+ previous: Option<&[QueryReport]>,
+ reports: &mut Vec<QueryReport>,
+ ) -> Result<()> {
+ let query = query_path
+ .file_stem()
+ .expect("query file has a filename")
+ .to_string_lossy()
+ .to_string();
+ let sql = fs::read_to_string(query_path)?;
+ let sql_parser_options = ctx.state().config_options().sql_parser.clone();
+ let statements = match sql_statements(&sql, &sql_parser_options) {
+ Ok(statements) => statements,
+ Err(error) => {
+ let report = QueryReport {
+ query,
+ statement: 1,
+ operators: vec![],
+ success: false,
+ error: Some(error.to_string()),
+ };
+ print_query_report(&report, previous);
+ reports.push(report);
+ return Ok(());
+ }
+ };
+ for (statement, sql) in statements.into_iter().enumerate() {
+ let statement = statement + 1;
+ let report = match self.report_statement(ctx, sql).await {
+ Ok(operators) => QueryReport {
+ query: query.clone(),
+ statement,
+ operators,
+ success: true,
+ error: None,
+ },
+ Err(error) => QueryReport {
+ query: query.clone(),
+ statement,
+ operators: vec![],
+ success: false,
+ error: Some(error.to_string()),
+ },
+ };
+ print_query_report(&report, previous);
+ reports.push(report);
+ }
+ Ok(())
+ }
+
fn report_path(&self, branch: &str) -> PathBuf {
let report_name = self.query.as_ref().map_or_else(
|| "statistics.json".to_string(),
@@ -772,6 +786,88 @@ mod tests {
assert!(!reports.is_empty());
}
+ async fn report_query_files(
+ options: &RunOpt,
+ ctx: &SessionContext,
+ ) -> Vec<QueryReport> {
+ let mut reports = vec![];
+ for path in query_files(&options.query_path, options.query.as_deref()).unwrap() {
+ options
+ .report_query_file(ctx, &path, None, &mut reports)
+ .await
+ .unwrap();
+ }
+ reports
+ }
+
+ #[tokio::test]
+ async fn refreshes_sql_dialect_between_query_files() {
+ let directory = tempdir().unwrap();
+ fs::write(
+ directory.path().join("01.sql"),
+ "SET datafusion.sql_parser.dialect = 'MySQL'",
+ )
+ .unwrap();
+ fs::write(directory.path().join("02.sql"), "# MySQL comment\nSELECT 1").unwrap();
+ fs::write(
+ directory.path().join("03.sql"),
+ "RESET datafusion.sql_parser.dialect",
+ )
+ .unwrap();
+ fs::write(directory.path().join("04.sql"), "SELECT 1,").unwrap();
+ let options = RunOpt {
+ query: None,
+ compare: None,
+ path: directory.path().to_path_buf(),
+ query_path: directory.path().to_path_buf(),
+ };
+
+ let reports = report_query_files(&options, &SessionContext::new()).await;
+
+ assert_eq!(reports.len(), 4);
+ assert!(reports.iter().all(|report| report.success));
+ assert_eq!(reports[1].query, "02");
+ assert!(!reports[1].operators.is_empty());
+ assert_eq!(reports[3].query, "04");
+ assert!(!reports[3].operators.is_empty());
+ }
+
+ #[tokio::test]
+ async fn refreshes_parser_recursion_limit_between_query_files() {
+ let directory = tempdir().unwrap();
+ fs::write(
+ directory.path().join("01.sql"),
+ "SET datafusion.sql_parser.recursion_limit = 2",
+ )
+ .unwrap();
+ fs::write(
+ directory.path().join("02.sql"),
+ "SELECT (((((((((((1)))))))))))",
+ )
+ .unwrap();
+ let options = RunOpt {
+ query: None,
+ compare: None,
+ path: directory.path().to_path_buf(),
+ query_path: directory.path().to_path_buf(),
+ };
+
+ let reports = report_query_files(&options, &SessionContext::new()).await;
+
+ assert_eq!(reports.len(), 2);
+ assert!(reports[0].success);
+ assert!(!reports[1].success);
+ assert_eq!(reports[1].query, "02");
+ assert!(
+ reports[1]
+ .error
+ .as_deref()
+ .is_some_and(|error| error.contains("RecursionLimitExceeded")),
+ "unexpected report: {:?}",
+ reports[1]
+ );
+ }
+
#[test]
fn persists_failed_reports() {
let directory = tempdir().unwrap();
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org