diff --git a/datafusion/core/tests/sql/sql_api.rs b/datafusion/core/tests/sql/sql_api.rs index ac77fa9312e7..ec086bcc50c7 100644 --- a/datafusion/core/tests/sql/sql_api.rs +++ b/datafusion/core/tests/sql/sql_api.rs @@ -33,7 +33,6 @@ async fn test_window_function() { w AS (ORDER BY t1.v1 ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW);"#, ) .await; - println!("{:?}", df); assert!(df.is_ok()); } diff --git a/datafusion/sql/src/select.rs b/datafusion/sql/src/select.rs index c67d6776c75b..ca4aa5b9e8ed 100644 --- a/datafusion/sql/src/select.rs +++ b/datafusion/sql/src/select.rs @@ -45,8 +45,8 @@ use datafusion_expr::{ use indexmap::IndexMap; use sqlparser::ast::{ - Distinct, Expr as SQLExpr, GroupByExpr, NamedWindowExpr, OrderByExpr, VisitMut, - VisitorMut, WildcardAdditionalOptions, WindowType, + visit_expressions_mut, Distinct, Expr as SQLExpr, GroupByExpr, NamedWindowExpr, + OrderByExpr, WildcardAdditionalOptions, WindowType, }; use sqlparser::ast::{NamedWindowDefinition, Select, SelectItem, TableWithJoins}; @@ -850,60 +850,46 @@ fn check_conflicting_windows(window_defs: &[NamedWindowDefinition]) -> Result<() Ok(()) } -// Visit the expression to find all window functions -struct WindowFunctionVisitor<'a> { - named_windows: &'a [NamedWindowDefinition], -} - -impl VisitorMut for WindowFunctionVisitor<'_> { - fn pre_visit_expr(&mut self, expr: &mut SQLExpr) -> ControlFlow { - if let SQLExpr::Function(f) = expr { - if let Some(WindowType::NamedWindow(ident)) = &f.over { - let ident = ident.clone(); - for NamedWindowDefinition(window_ident, window_expr) in - self.named_windows.iter() - { - if ident.eq(window_ident) { - f.over = Some(match window_expr { - NamedWindowExpr::NamedWindow(ident) => { - WindowType::NamedWindow(ident.clone()) - } - NamedWindowExpr::WindowSpec(spec) => { - WindowType::WindowSpec(spec.clone()) - } - }) - } - } - // All named windows must be defined with a WindowSpec. - if let Some(WindowType::NamedWindow(ident)) = &f.over { - return ControlFlow::Break(DataFusionError::Plan(format!( - "The window {ident} is not defined!" - ))); - } - } - } - ControlFlow::Continue(()) - } - - type Break = DataFusionError; -} - // If the projection is done over a named window, that window // name must be defined. Otherwise, it gives an error. fn match_window_definitions( projection: &mut [SelectItem], named_windows: &[NamedWindowDefinition], ) -> Result<()> { - if named_windows.is_empty() { - return Ok(()); - } for proj in projection.iter_mut() { if let SelectItem::ExprWithAlias { expr, alias: _ } | SelectItem::UnnamedExpr(expr) = proj { - let mut visitor = WindowFunctionVisitor { named_windows }; - - match VisitMut::visit(expr, &mut visitor) { + let result = visit_expressions_mut(expr, |expr| { + if let SQLExpr::Function(f) = expr { + if let Some(WindowType::NamedWindow(_)) = &f.over { + for NamedWindowDefinition(window_ident, window_expr) in + named_windows + { + if let Some(WindowType::NamedWindow(ident)) = &f.over { + if ident.eq(window_ident) { + f.over = Some(match window_expr { + NamedWindowExpr::NamedWindow(ident) => { + WindowType::NamedWindow(ident.clone()) + } + NamedWindowExpr::WindowSpec(spec) => { + WindowType::WindowSpec(spec.clone()) + } + }) + } + } + } + // All named windows must be defined with a WindowSpec. + if let Some(WindowType::NamedWindow(ident)) = &f.over { + return ControlFlow::Break(DataFusionError::Plan(format!( + "The window {ident} is not defined!" + ))); + } + } + } + ControlFlow::Continue(()) + }); + match result { ControlFlow::Continue(_) => (), ControlFlow::Break(err) => return Err(err), }; diff --git a/datafusion/sqllogictest/bin/sqllogictests.rs b/datafusion/sqllogictest/bin/sqllogictests.rs index bbb88819efe0..af10bbb7e73e 100644 --- a/datafusion/sqllogictest/bin/sqllogictests.rs +++ b/datafusion/sqllogictest/bin/sqllogictests.rs @@ -53,6 +53,7 @@ const SQLITE_PREFIX: &str = "sqlite"; pub fn main() -> Result<()> { tokio::runtime::Builder::new_multi_thread() + .thread_stack_size(4 * 1024 * 1024) .enable_all() .build()? .block_on(run_tests())