From e0f9dcbe65eb8ba9949e76e38f12b507661b8cea Mon Sep 17 00:00:00 2001 From: xudong963 Date: Sat, 14 Sep 2024 12:29:32 +0800 Subject: [PATCH] fix --- .../bind_table_function.rs | 50 ++++++++----------- .../rule/rewrite/rule_push_down_sort_scan.rs | 47 +++++++++++++---- .../mode/standalone/explain/lazy_read.test | 2 +- .../suites/mode/standalone/explain/limit.test | 2 +- 4 files changed, 59 insertions(+), 42 deletions(-) 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 cb3b26eed4e25..98b17a22025e2 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 @@ -45,7 +45,6 @@ use crate::binder::ColumnBindingBuilder; use crate::binder::Visibility; use crate::optimizer::SExpr; use crate::planner::semantic::normalize_identifier; -use crate::plans::BoundColumnRef; use crate::plans::EvalScalar; use crate::plans::FunctionCall; use crate::plans::RelOperator; @@ -248,28 +247,16 @@ impl Binder { }; if let Some(fields) = fields { - if let RelOperator::ProjectSet(plan) = (*srf_expr.plan).clone() { - if plan.srfs.len() != 1 { + if let RelOperator::EvalScalar(plan) = (*srf_expr.plan).clone() { + if plan.items.len() != 1 { return Err(ErrorCode::Internal(format!( - "Invalid table function subquery items, expect 1, but got {}", - plan.srfs.len() + "Invalid table function subquery EvalScalar items, expect 1, but got {}", + plan.items.len() ))); } // Delete srf result tuple column, extract tuple inner columns instead let _ = bind_context.columns.pop(); - let column = self.metadata.read().column(plan.srfs[0].index).clone(); - let column_binding = ColumnBindingBuilder::new( - column.name(), - column.index(), - Box::new(column.data_type().clone()), - Visibility::InVisible, - ) - .build(); - - let scalar = ScalarExpr::BoundColumnRef(BoundColumnRef { - span: None, - column: column_binding, - }); + let scalar = &plan.items[0].scalar; // Add tuple inner columns let mut items = Vec::with_capacity(fields.len()); @@ -303,7 +290,7 @@ impl Binder { } let eval_scalar = EvalScalar { items }; let new_expr = - SExpr::create_unary(Arc::new(eval_scalar.into()), Arc::new(srf_expr)); + SExpr::create_unary(Arc::new(eval_scalar.into()), srf_expr.children[0].clone()); if let Some(alias) = alias { bind_context.apply_table_alias(alias, &self.name_resolution_ctx)?; @@ -393,24 +380,27 @@ impl Binder { ) .build() }; + + let eval_scalar = EvalScalar { + items: vec![ScalarItem { + scalar: srf_result, + index: column_binding.index, + }], + }; // Add srf result column - bind_context.add_column_binding(column_binding.clone()); - let (mut new_expr, mut bind_context) = self + bind_context.add_column_binding(column_binding); + + let flatten_expr = + SExpr::create_unary(Arc::new(eval_scalar.into()), Arc::new(srf_expr)); + + let (new_expr, mut bind_context) = self .extract_srf_table_function_columns( &mut bind_context, span, &func_name, - srf_expr, + flatten_expr, alias, )?; - let eval_scalar = EvalScalar { - items: vec![ScalarItem { - scalar: srf_result, - index: column_binding.index, - }], - }; - new_expr = - SExpr::create_unary(Arc::new(eval_scalar.into()), Arc::new(new_expr)); // add left table columns. let mut new_columns = parent_context.columns.clone(); diff --git a/src/query/sql/src/planner/optimizer/rule/rewrite/rule_push_down_sort_scan.rs b/src/query/sql/src/planner/optimizer/rule/rewrite/rule_push_down_sort_scan.rs index 4c7992ff10ca6..bc17984429dab 100644 --- a/src/query/sql/src/planner/optimizer/rule/rewrite/rule_push_down_sort_scan.rs +++ b/src/query/sql/src/planner/optimizer/rule/rewrite/rule_push_down_sort_scan.rs @@ -24,7 +24,6 @@ use crate::optimizer::RuleID; use crate::optimizer::SExpr; use crate::plans::RelOp; use crate::plans::RelOperator; -use crate::plans::Scan; use crate::plans::Sort; /// Input: Sort @@ -44,13 +43,25 @@ impl RulePushDownSortScan { pub fn new() -> Self { Self { id: RuleID::PushDownSortScan, - matchers: vec![Matcher::MatchOp { - op_type: RelOp::Sort, - children: vec![Matcher::MatchOp { - op_type: RelOp::Scan, - children: vec![], - }], - }], + matchers: vec![ + Matcher::MatchOp { + op_type: RelOp::Sort, + children: vec![Matcher::MatchOp { + op_type: RelOp::Scan, + children: vec![], + }], + }, + Matcher::MatchOp { + op_type: RelOp::Sort, + children: vec![Matcher::MatchOp { + op_type: RelOp::EvalScalar, + children: vec![Matcher::MatchOp { + op_type: RelOp::Scan, + children: vec![], + }], + }], + }, + ], } } } @@ -63,16 +74,32 @@ impl Rule for RulePushDownSortScan { fn apply(&self, s_expr: &SExpr, state: &mut TransformResult) -> Result<()> { let sort: Sort = s_expr.plan().clone().try_into()?; let child = s_expr.child(0)?; - let mut get: Scan = child.plan().clone().try_into()?; + let mut get = match child.plan() { + RelOperator::Scan(scan) => scan.clone(), + RelOperator::EvalScalar(_) => { + let child = child.child(0)?; + child.plan().clone().try_into()? + } + _ => unreachable!(), + }; if get.order_by.is_none() { get.order_by = Some(sort.items); } if let Some(limit) = sort.limit { get.limit = Some(get.limit.map_or(limit, |c| cmp::max(c, limit))); } + let get = SExpr::create_leaf(Arc::new(RelOperator::Scan(get))); - let mut result = s_expr.replace_children(vec![Arc::new(get)]); + let mut result = match child.plan() { + RelOperator::Scan(_) => s_expr.replace_children(vec![Arc::new(get)]), + RelOperator::EvalScalar(_) => { + let child = child.replace_children(vec![Arc::new(get)]); + s_expr.replace_children(vec![Arc::new(child)]) + } + _ => unreachable!(), + }; + result.set_applied_rule(&self.id); state.add_result(result); Ok(()) diff --git a/tests/sqllogictests/suites/mode/standalone/explain/lazy_read.test b/tests/sqllogictests/suites/mode/standalone/explain/lazy_read.test index 9109687630c19..5f6ef467e377c 100644 --- a/tests/sqllogictests/suites/mode/standalone/explain/lazy_read.test +++ b/tests/sqllogictests/suites/mode/standalone/explain/lazy_read.test @@ -134,7 +134,7 @@ Limit ├── read size: 0 ├── partitions total: 0 ├── partitions scanned: 0 - ├── push downs: [filters: [], limit: NONE] + ├── push downs: [filters: [], limit: 2] └── estimated rows: 0.00 query T diff --git a/tests/sqllogictests/suites/mode/standalone/explain/limit.test b/tests/sqllogictests/suites/mode/standalone/explain/limit.test index a4f3ea9a09626..0b8bc375d6366 100644 --- a/tests/sqllogictests/suites/mode/standalone/explain/limit.test +++ b/tests/sqllogictests/suites/mode/standalone/explain/limit.test @@ -70,7 +70,7 @@ Limit ├── read size: < 1 KiB ├── partitions total: 1 ├── partitions scanned: 1 - ├── push downs: [filters: [], limit: NONE] + ├── push downs: [filters: [], limit: 8] └── estimated rows: 10.00 query T