From f02a5eec50fcff465e4ea9b545001a6c06a85c5d Mon Sep 17 00:00:00 2001 From: Kould <2435992353@qq.com> Date: Sat, 16 Mar 2024 20:38:37 +0800 Subject: [PATCH 1/3] fix: expression ownership in the temp table of subquery --- src/binder/expr.rs | 30 ++++++++++++++++++------------ src/binder/select.rs | 7 +++++-- src/expression/mod.rs | 12 ++++++++++++ 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/binder/expr.rs b/src/binder/expr.rs index b021c904..216cdfb8 100644 --- a/src/binder/expr.rs +++ b/src/binder/expr.rs @@ -110,13 +110,13 @@ impl<'a, T: Transaction> Binder<'a, T> { }), Expr::Subquery(subquery) => { let (sub_query, column) = self.bind_subquery(subquery)?; - self.context.sub_query(SubQueryType::SubQuery(sub_query)); - - if self.context.is_step(&QueryBindStep::Where) { - Ok(self.bind_temp_column(column)) + let (expr, sub_query) = if self.context.is_step(&QueryBindStep::Where) { + self.bind_temp_table(column, sub_query)? } else { - Ok(ScalarExpression::ColumnRef(column)) - } + (ScalarExpression::ColumnRef(column), sub_query) + }; + self.context.sub_query(SubQueryType::SubQuery(sub_query)); + Ok(expr) } Expr::InSubquery { expr, @@ -124,8 +124,6 @@ impl<'a, T: Transaction> Binder<'a, T> { negated, } => { let (sub_query, column) = self.bind_subquery(subquery)?; - self.context - .sub_query(SubQueryType::InSubQuery(*negated, sub_query)); if !self.context.is_step(&QueryBindStep::Where) { return Err(DatabaseError::UnsupportedStmt( @@ -133,7 +131,9 @@ impl<'a, T: Transaction> Binder<'a, T> { )); } - let alias_expr = self.bind_temp_column(column); + let (alias_expr, sub_query) = self.bind_temp_table(column, sub_query)?; + self.context + .sub_query(SubQueryType::InSubQuery(*negated, sub_query)); Ok(ScalarExpression::Binary { op: expression::BinaryOperator::Eq, @@ -203,16 +203,22 @@ impl<'a, T: Transaction> Binder<'a, T> { } } - fn bind_temp_column(&mut self, column: ColumnRef) -> ScalarExpression { + fn bind_temp_table( + &mut self, + column: ColumnRef, + sub_query: LogicalPlan, + ) -> Result<(ScalarExpression, LogicalPlan), DatabaseError> { let mut alias_column = ColumnCatalog::clone(&column); alias_column.set_table_name(self.context.temp_table()); - ScalarExpression::Alias { + let alias_expr = ScalarExpression::Alias { expr: Box::new(ScalarExpression::ColumnRef(column)), alias: AliasType::Expr(Box::new(ScalarExpression::ColumnRef(Arc::new( alias_column, )))), - } + }; + let alias_plan = self.bind_project(sub_query, vec![alias_expr.clone()])?; + Ok((alias_expr, alias_plan)) } fn bind_subquery( diff --git a/src/binder/select.rs b/src/binder/select.rs index 7e2da9d7..43beddd0 100644 --- a/src/binder/select.rs +++ b/src/binder/select.rs @@ -510,7 +510,7 @@ impl<'a, T: Transaction> Binder<'a, T> { Ok(FilterOperator::build(having, children, true)) } - fn bind_project( + pub(crate) fn bind_project( &mut self, children: LogicalPlan, select_list: Vec, @@ -722,7 +722,10 @@ impl<'a, T: Transaction> Binder<'a, T> { } => { match op { BinaryOperator::Eq => { - match (left_expr.unpack_alias_ref(), right_expr.unpack_alias_ref()) { + match ( + left_expr.unpack_alias_inner_ref().unpack_alias_ref(), + right_expr.unpack_alias_inner_ref().unpack_alias_ref(), + ) { // example: foo = bar (ScalarExpression::ColumnRef(l), ScalarExpression::ColumnRef(r)) => { // reorder left and right joins keys to pattern: (left, right) diff --git a/src/expression/mod.rs b/src/expression/mod.rs index 7c9cb659..ca833c4f 100644 --- a/src/expression/mod.rs +++ b/src/expression/mod.rs @@ -136,6 +136,18 @@ impl ScalarExpression { } } + pub fn unpack_alias_inner_ref(&self) -> &ScalarExpression { + if let ScalarExpression::Alias { + alias: AliasType::Expr(expr), + .. + } = self + { + expr.unpack_alias_inner_ref() + } else { + self + } + } + pub fn try_reference(&mut self, output_exprs: &[ScalarExpression]) { let fn_output_column = |expr: &ScalarExpression| expr.unpack_alias_ref().output_column(); let self_column = fn_output_column(self); From 3b71e7f11d69a29d0da9581edb0d51ebf59602fa Mon Sep 17 00:00:00 2001 From: Kould <2435992353@qq.com> Date: Sat, 16 Mar 2024 23:17:58 +0800 Subject: [PATCH 2/3] fix: bind error of the same fields in different tables in `in subquery` --- src/binder/expr.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/binder/expr.rs b/src/binder/expr.rs index 216cdfb8..f35a4afa 100644 --- a/src/binder/expr.rs +++ b/src/binder/expr.rs @@ -123,6 +123,7 @@ impl<'a, T: Transaction> Binder<'a, T> { subquery, negated, } => { + let left_expr = Box::new(self.bind_expr(expr)?); let (sub_query, column) = self.bind_subquery(subquery)?; if !self.context.is_step(&QueryBindStep::Where) { @@ -137,7 +138,7 @@ impl<'a, T: Transaction> Binder<'a, T> { Ok(ScalarExpression::Binary { op: expression::BinaryOperator::Eq, - left_expr: Box::new(self.bind_expr(expr)?), + left_expr, right_expr: Box::new(alias_expr), ty: LogicalType::Boolean, }) @@ -295,7 +296,7 @@ impl<'a, T: Transaction> Binder<'a, T> { } else { // handle col syntax let mut got_column = None; - for table_catalog in self.context.bind_table.values() { + for table_catalog in self.context.bind_table.values().rev() { if let Some(column_catalog) = table_catalog.get_column_by_name(&column_name) { got_column = Some(column_catalog); } From b74fff14d8a9b86363dd930ea9c8faaf413f6496 Mon Sep 17 00:00:00 2001 From: Kould <2435992353@qq.com> Date: Sun, 17 Mar 2024 02:30:15 +0800 Subject: [PATCH 3/3] fix: combine_operators.rs deletes the temp table alias of subquery --- src/binder/select.rs | 11 +++++----- src/expression/mod.rs | 22 +++++++++---------- .../rule/normalization/combine_operators.rs | 2 -- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/binder/select.rs b/src/binder/select.rs index 43beddd0..0832fb74 100644 --- a/src/binder/select.rs +++ b/src/binder/select.rs @@ -76,7 +76,7 @@ impl<'a, T: Transaction> Binder<'a, T> { // Resolve scalar function call. // TODO support SRF(Set-Returning Function). - let mut select_list = self.normalize_select_item(&select.projection)?; + let mut select_list = self.normalize_select_item(&select.projection, &plan)?; if let Some(predicate) = &select.selection { plan = self.bind_where(plan, predicate)?; @@ -341,6 +341,7 @@ impl<'a, T: Transaction> Binder<'a, T> { fn normalize_select_item( &mut self, items: &[SelectItem], + plan: &LogicalPlan, ) -> Result, DatabaseError> { let mut select_items = vec![]; @@ -359,6 +360,9 @@ impl<'a, T: Transaction> Binder<'a, T> { }); } SelectItem::Wildcard(_) => { + if let Operator::Project(op) = &plan.operator { + return Ok(op.exprs.clone()); + } for (table_name, _) in self.context.bind_table.keys() { self.bind_table_column_refs(&mut select_items, table_name.clone())?; } @@ -722,10 +726,7 @@ impl<'a, T: Transaction> Binder<'a, T> { } => { match op { BinaryOperator::Eq => { - match ( - left_expr.unpack_alias_inner_ref().unpack_alias_ref(), - right_expr.unpack_alias_inner_ref().unpack_alias_ref(), - ) { + match (left_expr.unpack_alias_ref(), right_expr.unpack_alias_ref()) { // example: foo = bar (ScalarExpression::ColumnRef(l), ScalarExpression::ColumnRef(r)) => { // reorder left and right joins keys to pattern: (left, right) diff --git a/src/expression/mod.rs b/src/expression/mod.rs index ca833c4f..78672a4c 100644 --- a/src/expression/mod.rs +++ b/src/expression/mod.rs @@ -121,7 +121,13 @@ pub enum ScalarExpression { impl ScalarExpression { pub fn unpack_alias(self) -> ScalarExpression { - if let ScalarExpression::Alias { expr, .. } = self { + if let ScalarExpression::Alias { + alias: AliasType::Expr(expr), + .. + } = self + { + expr.unpack_alias() + } else if let ScalarExpression::Alias { expr, .. } = self { expr.unpack_alias() } else { self @@ -129,27 +135,21 @@ impl ScalarExpression { } pub fn unpack_alias_ref(&self) -> &ScalarExpression { - if let ScalarExpression::Alias { expr, .. } = self { - expr.unpack_alias_ref() - } else { - self - } - } - - pub fn unpack_alias_inner_ref(&self) -> &ScalarExpression { if let ScalarExpression::Alias { alias: AliasType::Expr(expr), .. } = self { - expr.unpack_alias_inner_ref() + expr.unpack_alias_ref() + } else if let ScalarExpression::Alias { expr, .. } = self { + expr.unpack_alias_ref() } else { self } } pub fn try_reference(&mut self, output_exprs: &[ScalarExpression]) { - let fn_output_column = |expr: &ScalarExpression| expr.unpack_alias_ref().output_column(); + let fn_output_column = |expr: &ScalarExpression| expr.output_column(); let self_column = fn_output_column(self); if let Some((pos, _)) = output_exprs .iter() diff --git a/src/optimizer/rule/normalization/combine_operators.rs b/src/optimizer/rule/normalization/combine_operators.rs index 785b3106..2b59760b 100644 --- a/src/optimizer/rule/normalization/combine_operators.rs +++ b/src/optimizer/rule/normalization/combine_operators.rs @@ -61,8 +61,6 @@ impl NormalizationRule for CollapseProject { if let Operator::Project(child_op) = graph.operator(child_id) { if is_subset_exprs(&op.exprs, &child_op.exprs) { graph.remove_node(child_id, false); - } else { - graph.remove_node(node_id, false); } } }