diff --git a/src/query/functions/src/lib.rs b/src/query/functions/src/lib.rs index 0710fdcedd42..9cd40ebb468b 100644 --- a/src/query/functions/src/lib.rs +++ b/src/query/functions/src/lib.rs @@ -71,6 +71,9 @@ pub const GENERAL_WINDOW_FUNCTIONS: [&str; 13] = [ "cume_dist", ]; +pub const RANK_WINDOW_FUNCTIONS: [&str; 5] = + ["first_value", "first", "last_value", "last", "nth_value"]; + pub const GENERAL_LAMBDA_FUNCTIONS: [&str; 16] = [ "array_transform", "array_apply", diff --git a/src/query/sql/src/planner/binder/aggregate.rs b/src/query/sql/src/planner/binder/aggregate.rs index 9433c6c4ee43..151608950d4e 100644 --- a/src/query/sql/src/planner/binder/aggregate.rs +++ b/src/query/sql/src/planner/binder/aggregate.rs @@ -33,7 +33,7 @@ use itertools::Itertools; use super::prune_by_children; use super::ExprContext; use super::Finder; -use crate::binder::project_set::SetReturningRewriter; +use crate::binder::project_set::SetReturningAnalyzer; use crate::binder::scalar::ScalarBinder; use crate::binder::select::SelectList; use crate::binder::Binder; @@ -349,13 +349,6 @@ impl Binder { bind_context: &mut BindContext, select_list: &mut SelectList, ) -> Result<()> { - if !bind_context.srf_info.srfs.is_empty() { - // Rewrite the Set-returning functions in Aggregate function as columns. - let mut srf_rewriter = SetReturningRewriter::new(bind_context, true); - for item in select_list.items.iter_mut() { - srf_rewriter.visit(&mut item.scalar)?; - } - } let mut rewriter = AggregateRewriter::new(bind_context, self.metadata.clone()); for item in select_list.items.iter_mut() { rewriter.visit(&mut item.scalar)?; @@ -714,6 +707,9 @@ impl Binder { .bind(expr) .or_else(|e| self.resolve_alias_item(bind_context, expr, available_aliases, e))?; + let mut analyzer = SetReturningAnalyzer::new(bind_context, self.metadata.clone()); + analyzer.visit(&mut scalar_expr)?; + if collect_grouping_sets && !grouping_sets.last().unwrap().contains(&scalar_expr) { grouping_sets.last_mut().unwrap().push(scalar_expr.clone()); } @@ -727,11 +723,6 @@ impl Binder { continue; } - if !bind_context.srf_info.srfs.is_empty() { - let mut srf_rewriter = SetReturningRewriter::new(bind_context, false); - srf_rewriter.visit(&mut scalar_expr)?; - } - let group_item_name = format!("{:#}", expr); let index = if let ScalarExpr::BoundColumnRef(BoundColumnRef { column: ColumnBinding { index, .. }, @@ -877,12 +868,7 @@ impl Binder { .set_span(expr.span()), ) } else { - let (alias, mut scalar) = available_aliases[result[0]].clone(); - - if !bind_context.srf_info.srfs.is_empty() { - let mut srf_rewriter = SetReturningRewriter::new(bind_context, false); - srf_rewriter.visit(&mut scalar)?; - } + let (alias, scalar) = available_aliases[result[0]].clone(); // check scalar first, avoid duplicate create column. let mut scalar_column_index = None; diff --git a/src/query/sql/src/planner/binder/bind_query/bind_select.rs b/src/query/sql/src/planner/binder/bind_query/bind_select.rs index ff28a910dae6..5645b593c683 100644 --- a/src/query/sql/src/planner/binder/bind_query/bind_select.rs +++ b/src/query/sql/src/planner/binder/bind_query/bind_select.rs @@ -141,10 +141,10 @@ impl Binder { .map(|item| (item.alias.clone(), item.scalar.clone())) .collect::>(); - // Check Set-returning functions, if the argument contains aggregation function or group item, + // Rewrite Set-returning functions, if the argument contains aggregation function or group item, // set as lazy Set-returning functions. if !from_context.srf_info.srfs.is_empty() { - self.check_project_set_select(&mut from_context)?; + self.rewrite_project_set_select(&mut from_context)?; } // Bind Set-returning functions before filter plan and aggregate plan. diff --git a/src/query/sql/src/planner/binder/bind_table_reference/bind_table_function.rs b/src/query/sql/src/planner/binder/bind_table_reference/bind_table_function.rs index 886627804681..30eb9e5e889a 100644 --- a/src/query/sql/src/planner/binder/bind_table_reference/bind_table_function.rs +++ b/src/query/sql/src/planner/binder/bind_table_reference/bind_table_function.rs @@ -38,7 +38,6 @@ use databend_common_storages_result_cache::ResultCacheMetaManager; use databend_common_storages_result_cache::ResultScan; use databend_common_users::UserApiProvider; -use crate::binder::project_set::SetReturningRewriter; use crate::binder::scalar::ScalarBinder; use crate::binder::table_args::bind_table_args; use crate::binder::Binder; @@ -50,7 +49,6 @@ use crate::plans::EvalScalar; use crate::plans::FunctionCall; use crate::plans::RelOperator; use crate::plans::ScalarItem; -use crate::plans::VisitorMut; use crate::BindContext; use crate::ScalarExpr; @@ -356,11 +354,6 @@ impl Binder { self.normalize_select_list(&mut bind_context, &select_list)?; // analyze Set-returning functions. self.analyze_project_set_select(&mut bind_context, &mut select_list)?; - // rewrite Set-returning functions as columns. - let mut srf_rewriter = SetReturningRewriter::new(&mut bind_context, false); - for item in select_list.items.iter_mut() { - srf_rewriter.visit(&mut item.scalar)?; - } // bind Set-returning functions. let srf_expr = self.bind_project_set(&mut bind_context, child, false)?; // clear Set-returning functions, avoid duplicate bind. diff --git a/src/query/sql/src/planner/binder/distinct.rs b/src/query/sql/src/planner/binder/distinct.rs index df1d26e6cedd..3e064d9567dc 100644 --- a/src/query/sql/src/planner/binder/distinct.rs +++ b/src/query/sql/src/planner/binder/distinct.rs @@ -18,7 +18,6 @@ use std::sync::Arc; use databend_common_ast::Span; use databend_common_exception::Result; -use crate::binder::project_set::SetReturningRewriter; use crate::binder::Binder; use crate::binder::ColumnBinding; use crate::optimizer::SExpr; @@ -43,14 +42,6 @@ impl Binder { scalar_items: &mut HashMap, child: SExpr, ) -> Result { - if !bind_context.srf_info.srfs.is_empty() { - // Rewrite the Set-returning functions as columns. - let mut srf_rewriter = SetReturningRewriter::new(bind_context, false); - for (_, item) in scalar_items.iter_mut() { - srf_rewriter.visit(&mut item.scalar)?; - } - } - let scalar_items: Vec = scalar_items .drain() .map(|(_, item)| { diff --git a/src/query/sql/src/planner/binder/project_set.rs b/src/query/sql/src/planner/binder/project_set.rs index c507953b394e..29818a3162d6 100644 --- a/src/query/sql/src/planner/binder/project_set.rs +++ b/src/query/sql/src/planner/binder/project_set.rs @@ -17,22 +17,24 @@ use std::collections::HashSet; use std::mem; use std::sync::Arc; -use databend_common_exception::ErrorCode; use databend_common_exception::Result; use databend_common_expression::FunctionKind; use databend_common_functions::BUILTIN_FUNCTIONS; +use crate::binder::aggregate::AggregateRewriter; use crate::binder::select::SelectList; use crate::binder::ColumnBindingBuilder; use crate::format_scalar; use crate::optimizer::SExpr; use crate::plans::walk_expr_mut; use crate::plans::BoundColumnRef; +use crate::plans::FunctionCall; use crate::plans::ProjectSet; use crate::plans::ScalarItem; use crate::plans::VisitorMut; use crate::BindContext; use crate::Binder; +use crate::ColumnBinding; use crate::MetadataRef; use crate::ScalarExpr; use crate::Visibility; @@ -50,83 +52,23 @@ pub struct SetReturningInfo { pub lazy_srf_set: HashSet, } -/// Rewrite Set-returning functions as a BoundColumnRef. -pub(crate) struct SetReturningRewriter<'a> { - pub(crate) bind_context: &'a mut BindContext, - // Whether only rewrite the argument of aggregate function. - only_agg: bool, - // Whether current function is the argument of aggregate function. - is_agg_arg: bool, -} - -impl<'a> SetReturningRewriter<'a> { - pub(crate) fn new(bind_context: &'a mut BindContext, only_agg: bool) -> Self { - Self { - bind_context, - only_agg, - is_agg_arg: false, - } - } -} - -impl<'a> VisitorMut<'a> for SetReturningRewriter<'a> { - fn visit(&mut self, expr: &'a mut ScalarExpr) -> Result<()> { - match expr { - ScalarExpr::FunctionCall(func) => { - if (self.is_agg_arg || !self.only_agg) - && BUILTIN_FUNCTIONS - .get_property(&func.func_name) - .map(|property| property.kind == FunctionKind::SRF) - .unwrap_or(false) - { - let srf_display_name = format_scalar(expr); - if let Some(index) = self.bind_context.srf_info.srfs_map.get(&srf_display_name) - { - let srf_item = &self.bind_context.srf_info.srfs[*index]; - - let column_binding = ColumnBindingBuilder::new( - srf_display_name, - srf_item.index, - Box::new(srf_item.scalar.data_type()?), - Visibility::InVisible, - ) - .build(); - *expr = BoundColumnRef { - span: None, - column: column_binding, - } - .into(); - - return Ok(()); - } - return Err(ErrorCode::Internal("Invalid Set-returning function")); - } - } - ScalarExpr::AggregateFunction(_) => { - self.is_agg_arg = true; - } - _ => {} - } - walk_expr_mut(self, expr)?; - - self.is_agg_arg = false; - Ok(()) - } -} - /// Analyze Set-returning functions and create derived columns. -struct SetReturningAnalyzer<'a> { +pub(crate) struct SetReturningAnalyzer<'a> { bind_context: &'a mut BindContext, metadata: MetadataRef, } impl<'a> SetReturningAnalyzer<'a> { - fn new(bind_context: &'a mut BindContext, metadata: MetadataRef) -> Self { + pub(crate) fn new(bind_context: &'a mut BindContext, metadata: MetadataRef) -> Self { Self { bind_context, metadata, } } + + fn as_aggregate_rewriter(&mut self) -> AggregateRewriter { + AggregateRewriter::new(self.bind_context, self.metadata.clone()) + } } impl<'a> VisitorMut<'a> for SetReturningAnalyzer<'a> { @@ -137,22 +79,66 @@ impl<'a> VisitorMut<'a> for SetReturningAnalyzer<'a> { .map(|property| property.kind == FunctionKind::SRF) .unwrap_or(false) { - let srf_display_name = format_scalar(expr); + let mut replaced_args = Vec::with_capacity(func.arguments.len()); + for arg in func.arguments.iter() { + let mut arg = arg.clone(); + let mut aggregate_rewriter = self.as_aggregate_rewriter(); + aggregate_rewriter.visit(&mut arg)?; + replaced_args.push(arg); + } + + let replaced_expr: ScalarExpr = FunctionCall { + span: func.span, + func_name: func.func_name.clone(), + params: func.params.clone(), + arguments: replaced_args, + } + .into(); + + let srf_display_name = format_scalar(&replaced_expr); + + let srf_info = &mut self.bind_context.srf_info; + if let Some(column_binding) = + find_replaced_set_returning_function(srf_info, &srf_display_name) + { + *expr = BoundColumnRef { + span: None, + column: column_binding, + } + .into(); + return Ok(()); + } + let index = self.metadata.write().add_derived_column( srf_display_name.clone(), - expr.data_type()?, - Some(expr.clone()), + replaced_expr.data_type()?, + Some(replaced_expr.clone()), ); // Add the srf to bind context, build ProjectSet plan later. self.bind_context.srf_info.srfs.push(ScalarItem { index, - scalar: expr.clone(), + scalar: replaced_expr.clone(), }); - self.bind_context - .srf_info - .srfs_map - .insert(srf_display_name, self.bind_context.srf_info.srfs.len() - 1); + self.bind_context.srf_info.srfs_map.insert( + srf_display_name.clone(), + self.bind_context.srf_info.srfs.len() - 1, + ); + + let column_binding = ColumnBindingBuilder::new( + srf_display_name, + index, + Box::new(replaced_expr.data_type()?), + Visibility::Visible, + ) + .build(); + + *expr = BoundColumnRef { + span: None, + column: column_binding, + } + .into(); + return Ok(()); } } @@ -162,22 +148,22 @@ impl<'a> VisitorMut<'a> for SetReturningAnalyzer<'a> { } /// Check whether the argument of Set-returning functions contains aggregation function or group item. -/// If true, we need to lazy build `ProjectSet` plan -struct SetReturningChecker<'a> { +/// If true, rewrite aggregation function as a BoundColumnRef, and build a lazy `ProjectSet` plan +struct SetReturningRewriter<'a> { bind_context: &'a mut BindContext, - has_aggregate_argument: bool, + is_lazy_srf: bool, } -impl<'a> SetReturningChecker<'a> { +impl<'a> SetReturningRewriter<'a> { fn new(bind_context: &'a mut BindContext) -> Self { Self { bind_context, - has_aggregate_argument: false, + is_lazy_srf: false, } } } -impl<'a> VisitorMut<'a> for SetReturningChecker<'a> { +impl<'a> VisitorMut<'a> for SetReturningRewriter<'a> { fn visit(&mut self, expr: &'a mut ScalarExpr) -> Result<()> { if self .bind_context @@ -185,11 +171,11 @@ impl<'a> VisitorMut<'a> for SetReturningChecker<'a> { .group_items_map .contains_key(expr) { - self.has_aggregate_argument = true; + self.is_lazy_srf = true; } if let ScalarExpr::AggregateFunction(agg_func) = expr { - self.has_aggregate_argument = true; + self.is_lazy_srf = true; if let Some(index) = self .bind_context .aggregate_info @@ -235,23 +221,22 @@ impl Binder { Ok(()) } - pub(crate) fn check_project_set_select( + /// Rewrite the argument of project sets and find lazy srfs. + /// See [`SetReturningRewriter`] for more details. + pub(crate) fn rewrite_project_set_select( &mut self, bind_context: &mut BindContext, ) -> Result<()> { let mut srf_info = mem::take(&mut bind_context.srf_info); - let mut checker = SetReturningChecker::new(bind_context); + let mut rewriter = SetReturningRewriter::new(bind_context); for srf_item in srf_info.srfs.iter_mut() { let srf_display_name = format_scalar(&srf_item.scalar); - checker.has_aggregate_argument = false; - checker.visit(&mut srf_item.scalar)?; + rewriter.is_lazy_srf = false; + rewriter.visit(&mut srf_item.scalar)?; // If the argument contains aggregation function or group item. // add the srf index to lazy set. - if checker.has_aggregate_argument { - // srf_display_names.push(srf_display_name); - // if let Some(index) = bind_context.srf_info.srfs_map.get(&srf_display_name) { - // bind_context.srf_info.lazy_srf_set.insert(*index); + if rewriter.is_lazy_srf { if let Some(index) = srf_info.srfs_map.get(&srf_display_name) { srf_info.lazy_srf_set.insert(*index); } @@ -292,3 +277,21 @@ impl Binder { Ok(new_expr) } } + +/// Replace [`SetReturningFunction`] with a [`ColumnBinding`] if the function is already replaced. +pub fn find_replaced_set_returning_function( + srf_info: &SetReturningInfo, + srf_display_name: &str, +) -> Option { + srf_info.srfs_map.get(srf_display_name).map(|i| { + // This expression is already replaced. + let scalar_item = &srf_info.srfs[*i]; + ColumnBindingBuilder::new( + srf_display_name.to_string(), + scalar_item.index, + Box::new(scalar_item.scalar.data_type().unwrap()), + Visibility::Visible, + ) + .build() + }) +} diff --git a/src/query/sql/src/planner/binder/select.rs b/src/query/sql/src/planner/binder/select.rs index 3c3fd840bf3a..256b42fdd942 100644 --- a/src/query/sql/src/planner/binder/select.rs +++ b/src/query/sql/src/planner/binder/select.rs @@ -33,7 +33,6 @@ use databend_common_functions::BUILTIN_FUNCTIONS; use super::sort::OrderItem; use super::Finder; use crate::binder::bind_table_reference::JoinConditions; -use crate::binder::project_set::SetReturningRewriter; use crate::binder::scalar_common::split_conjunctions; use crate::binder::ColumnBindingBuilder; use crate::binder::ExprContext; @@ -50,7 +49,6 @@ use crate::plans::ScalarExpr; use crate::plans::ScalarItem; use crate::plans::UnionAll; use crate::plans::Visitor as _; -use crate::plans::VisitorMut; use crate::ColumnEntry; use crate::IndexType; use crate::Visibility; @@ -86,14 +84,7 @@ impl Binder { self.metadata.clone(), aliases, ); - let (mut scalar, _) = scalar_binder.bind(expr)?; - - // rewrite Set-returning functions as columns. - if !bind_context.srf_info.srfs.is_empty() { - let mut srf_rewriter = SetReturningRewriter::new(bind_context, false); - srf_rewriter.visit(&mut scalar)?; - } - + let (scalar, _) = scalar_binder.bind(expr)?; let f = |scalar: &ScalarExpr| { matches!( scalar, diff --git a/src/query/sql/src/planner/expression_parser.rs b/src/query/sql/src/planner/expression_parser.rs index e6d7b20b87ac..9e16429a6d00 100644 --- a/src/query/sql/src/planner/expression_parser.rs +++ b/src/query/sql/src/planner/expression_parser.rs @@ -257,6 +257,12 @@ pub fn parse_default_expr_to_string( )?; let (mut scalar, data_type) = *type_checker.resolve(ast)?; + if !scalar.evaluable() { + return Err(ErrorCode::SemanticError(format!( + "default value expression `{:#}` is invalid", + ast + ))); + } let schema_data_type = DataType::from(field.data_type()); if data_type != schema_data_type { scalar = wrap_cast(&scalar, &schema_data_type); @@ -314,6 +320,12 @@ pub fn parse_computed_expr_to_string( )?; let (scalar, data_type) = *type_checker.resolve(ast)?; + if !scalar.evaluable() { + return Err(ErrorCode::SemanticError(format!( + "computed column expression `{:#}` is invalid", + ast + ))); + } if data_type != DataType::from(field.data_type()) { return Err(ErrorCode::SemanticError(format!( "expected computed column expression have type {}, but `{}` has type {}.", diff --git a/src/query/sql/src/planner/plans/scalar_expr.rs b/src/query/sql/src/planner/plans/scalar_expr.rs index d7d165cfb8c5..25d96b69a2e1 100644 --- a/src/query/sql/src/planner/plans/scalar_expr.rs +++ b/src/query/sql/src/planner/plans/scalar_expr.rs @@ -25,8 +25,10 @@ use databend_common_exception::ErrorCode; use databend_common_exception::Result; use databend_common_expression::types::DataType; use databend_common_expression::types::NumberScalar; +use databend_common_expression::FunctionKind; use databend_common_expression::RemoteExpr; use databend_common_expression::Scalar; +use databend_common_functions::BUILTIN_FUNCTIONS; use databend_common_meta_app::schema::GetSequenceNextValueReq; use databend_common_meta_app::schema::SequenceIdent; use databend_common_meta_app::tenant::Tenant; @@ -208,6 +210,20 @@ impl ScalarExpr { } impl<'a> Visitor<'a> for EvaluableVisitor { + fn visit_function_call(&mut self, func: &'a FunctionCall) -> Result<()> { + if BUILTIN_FUNCTIONS + .get_property(&func.func_name) + .map(|property| property.kind == FunctionKind::SRF) + .unwrap_or(false) + { + self.evaluable = false; + } else { + for expr in &func.arguments { + self.visit(expr)?; + } + } + Ok(()) + } fn visit_window_function(&mut self, _: &'a WindowFunc) -> Result<()> { self.evaluable = false; Ok(()) diff --git a/src/query/sql/src/planner/semantic/grouping_check.rs b/src/query/sql/src/planner/semantic/grouping_check.rs index f5b3cce783ac..bb061fbde8bb 100644 --- a/src/query/sql/src/planner/semantic/grouping_check.rs +++ b/src/query/sql/src/planner/semantic/grouping_check.rs @@ -14,12 +14,9 @@ use databend_common_exception::ErrorCode; use databend_common_exception::Result; -use databend_common_expression::FunctionKind; -use databend_common_functions::BUILTIN_FUNCTIONS; use crate::binder::ColumnBindingBuilder; use crate::binder::Visibility; -use crate::format_scalar; use crate::plans::walk_expr_mut; use crate::plans::BoundColumnRef; use crate::plans::ScalarExpr; @@ -44,25 +41,6 @@ impl<'a> GroupingChecker<'a> { impl<'a> VisitorMut<'_> for GroupingChecker<'a> { fn visit(&mut self, expr: &mut ScalarExpr) -> Result<()> { - if !self - .bind_context - .aggregate_info - .group_items_map - .contains_key(expr) - { - if let ScalarExpr::FunctionCall(func) = expr { - // The srf returns a tuple type column, and a `get` function - // is always used to extract the inner column. - // Try rewrite the srf to a column first, so that we can - // check if the expr is in the `group_items_map`. - if func.func_name == "get" { - for arg in &mut func.arguments { - self.visit(arg)?; - } - } - } - } - if let Some(index) = self.bind_context.aggregate_info.group_items_map.get(expr) { let column = &self.bind_context.aggregate_info.group_items[*index]; let mut column_binding = if let ScalarExpr::BoundColumnRef(column_ref) = &column.scalar @@ -159,35 +137,6 @@ impl<'a> VisitorMut<'_> for GroupingChecker<'a> { return Err(ErrorCode::Internal("Invalid aggregate function")); } - ScalarExpr::FunctionCall(func) => { - if BUILTIN_FUNCTIONS - .get_property(&func.func_name) - .map(|property| property.kind == FunctionKind::SRF) - .unwrap_or(false) - { - let srf_display_name = format_scalar(expr); - if let Some(index) = self.bind_context.srf_info.srfs_map.get(&srf_display_name) - { - // Rewrite srf function as a column. - let srf_item = &self.bind_context.srf_info.srfs[*index]; - - let column_binding = ColumnBindingBuilder::new( - srf_display_name, - srf_item.index, - Box::new(srf_item.scalar.data_type()?), - Visibility::Visible, - ) - .build(); - *expr = BoundColumnRef { - span: None, - column: column_binding, - } - .into(); - return Ok(()); - } - return Err(ErrorCode::Internal("Invalid Set-returning function")); - } - } ScalarExpr::BoundColumnRef(column_ref) => { if let Some(index) = self .bind_context diff --git a/src/query/sql/src/planner/semantic/type_check.rs b/src/query/sql/src/planner/semantic/type_check.rs index 7e9341a4a22a..8e66b43735c8 100644 --- a/src/query/sql/src/planner/semantic/type_check.rs +++ b/src/query/sql/src/planner/semantic/type_check.rs @@ -91,6 +91,7 @@ use databend_common_functions::BUILTIN_FUNCTIONS; use databend_common_functions::GENERAL_LAMBDA_FUNCTIONS; use databend_common_functions::GENERAL_SEARCH_FUNCTIONS; use databend_common_functions::GENERAL_WINDOW_FUNCTIONS; +use databend_common_functions::RANK_WINDOW_FUNCTIONS; use databend_common_meta_app::principal::LambdaUDF; use databend_common_meta_app::principal::UDFDefinition; use databend_common_meta_app::principal::UDFScript; @@ -800,8 +801,8 @@ impl<'a> TypeChecker<'a> { .set_span(*span)); } let window = window.as_ref().unwrap(); - let rank_window = ["first_value", "first", "last_value", "last", "nth_value"]; - if !rank_window.contains(&func_name) && window.ignore_nulls.is_some() { + if !RANK_WINDOW_FUNCTIONS.contains(&func_name) && window.ignore_nulls.is_some() + { return Err(ErrorCode::SemanticError(format!( "window function {func_name} not support IGNORE/RESPECT NULLS option" )) @@ -2593,12 +2594,6 @@ impl<'a> TypeChecker<'a> { ) .set_span(span)); } - if !matches!(self.bind_context.expr_context, ExprContext::SelectClause) { - return Err(ErrorCode::SemanticError( - "set-returning functions can only be used in SELECT".to_string(), - ) - .set_span(span)); - } let original_context = self.bind_context.expr_context.clone(); self.bind_context diff --git a/src/query/sql/src/planner/semantic/window_check.rs b/src/query/sql/src/planner/semantic/window_check.rs index 8ccc5787c2d6..803f02e7121d 100644 --- a/src/query/sql/src/planner/semantic/window_check.rs +++ b/src/query/sql/src/planner/semantic/window_check.rs @@ -14,11 +14,8 @@ use databend_common_exception::ErrorCode; use databend_common_exception::Result; -use databend_common_expression::FunctionKind; -use databend_common_functions::BUILTIN_FUNCTIONS; use crate::binder::ColumnBindingBuilder; -use crate::format_scalar; use crate::plans::walk_expr_mut; use crate::plans::BoundColumnRef; use crate::plans::SubqueryExpr; @@ -39,62 +36,30 @@ impl<'a> WindowChecker<'a> { impl<'a> VisitorMut<'_> for WindowChecker<'a> { fn visit(&mut self, expr: &mut ScalarExpr) -> Result<()> { - match expr { - ScalarExpr::WindowFunction(win) => { - if let Some(column) = self - .bind_context - .windows - .window_functions_map - .get(&win.display_name) - { - let window_info = &self.bind_context.windows.window_functions[*column]; - let column_binding = ColumnBindingBuilder::new( - win.display_name.clone(), - window_info.index, - Box::new(window_info.func.return_type()), - Visibility::Visible, - ) - .build(); - *expr = BoundColumnRef { - span: None, - column: column_binding, - } - .into(); - return Ok(()); + if let ScalarExpr::WindowFunction(win) = expr { + if let Some(column) = self + .bind_context + .windows + .window_functions_map + .get(&win.display_name) + { + let window_info = &self.bind_context.windows.window_functions[*column]; + let column_binding = ColumnBindingBuilder::new( + win.display_name.clone(), + window_info.index, + Box::new(window_info.func.return_type()), + Visibility::Visible, + ) + .build(); + *expr = BoundColumnRef { + span: None, + column: column_binding, } - - return Err(ErrorCode::Internal("Window Check: Invalid window function")); + .into(); + return Ok(()); } - ScalarExpr::FunctionCall(func) => { - if BUILTIN_FUNCTIONS - .get_property(&func.func_name) - .map(|property| property.kind == FunctionKind::SRF) - .unwrap_or(false) - { - let srf_display_name = format_scalar(expr); - if let Some(index) = self.bind_context.srf_info.srfs_map.get(&srf_display_name) - { - // Rewrite srf function as a column. - let srf_item = &self.bind_context.srf_info.srfs[*index]; - let column_binding = ColumnBindingBuilder::new( - srf_display_name, - srf_item.index, - Box::new(srf_item.scalar.data_type()?), - Visibility::Visible, - ) - .build(); - *expr = BoundColumnRef { - span: None, - column: column_binding, - } - .into(); - return Ok(()); - } - return Err(ErrorCode::Internal("Invalid Set-returning function")); - } - } - _ => {} + return Err(ErrorCode::Internal("Window Check: Invalid window function")); } walk_expr_mut(self, expr) diff --git a/tests/sqllogictests/suites/base/03_common/03_0003_select_group_by.test b/tests/sqllogictests/suites/base/03_common/03_0003_select_group_by.test index b2e84947356b..f4a7184e3b9d 100644 --- a/tests/sqllogictests/suites/base/03_common/03_0003_select_group_by.test +++ b/tests/sqllogictests/suites/base/03_common/03_0003_select_group_by.test @@ -275,6 +275,20 @@ a1 a2 a3 +query TT +SELECT t.col1 AS col1, unnest(split(t.col2, ',')) AS col2 FROM t_str AS t GROUP BY t.col1, unnest(split(t.col2, ',')) ORDER BY col2; +---- +test a1 +test a2 +test a3 + +query T +SELECT t.col1 AS col1 FROM t_str AS t GROUP BY t.col1, unnest(split(t.col2, ',')); +---- +test +test +test + statement ok DROP TABLE t_str diff --git a/tests/sqllogictests/suites/base/09_fuse_engine/09_0000_remote_create_table.test b/tests/sqllogictests/suites/base/09_fuse_engine/09_0000_remote_create_table.test index 583ce134bdbf..771ca42c2576 100644 --- a/tests/sqllogictests/suites/base/09_fuse_engine/09_0000_remote_create_table.test +++ b/tests/sqllogictests/suites/base/09_fuse_engine/09_0000_remote_create_table.test @@ -36,7 +36,7 @@ CREATE TABLE t(c int) Engine = fuse snapshot_loc = 1 statement error 1301 CREATE TABLE t(c int) Engine = fuse SNAPSHOT_LOC = 1 -statement error 1065 +statement error 1081 CREATE TABLE t(id Int null, arr VARIANT null) cluster by(json_path_query(arr, '$[*][1]')) statement error 1081 diff --git a/tests/sqllogictests/suites/mode/standalone/explain/project_set.test b/tests/sqllogictests/suites/mode/standalone/explain/project_set.test index 73926f42045e..c4306e5f4f80 100644 --- a/tests/sqllogictests/suites/mode/standalone/explain/project_set.test +++ b/tests/sqllogictests/suites/mode/standalone/explain/project_set.test @@ -114,8 +114,8 @@ query T EXPLAIN SELECT json_each(t.b), unnest(t.b) FROM t ---- EvalScalar -├── output columns: [json_each(t.b) (#4), unnest(t.b) (#5)] -├── expressions: [json_each(t.b (#1)) (#2), get(1)(unnest(t.b (#1)) (#3))] +├── output columns: [json_each(t.b (#1)) (#2), unnest(t.b) (#4)] +├── expressions: [get(1)(unnest(t.b (#1)) (#3))] ├── estimated rows: 0.00 └── ProjectSet ├── output columns: [json_each(t.b (#1)) (#2), unnest(t.b (#1)) (#3)]