refactor(sql): stabilize binder alias semantics across aggregate, window, and clause rewrites

src/query/ast/src/parser/parser.rs

@@ -16,17 +16,7 @@ use nom::Parser;
 
 use crate::ParseError;
 use crate::Result;
-use crate::ast::DatabaseRef;
-use crate::ast::Expr;
-use crate::ast::Identifier;
-use crate::ast::Literal;
-use crate::ast::ProcedureIdentity;
-use crate::ast::Query;
-use crate::ast::SelectTarget;
-use crate::ast::SetExpr;
-use crate::ast::Statement;
-use crate::ast::StatementWithFormat;
-use crate::ast::TableRef;
+use crate::ast::*;
 use crate::parser::Backtrace;
 use crate::parser::common::IResult;
 use crate::parser::common::comma_separated_list0;
@@ -48,9 +38,6 @@ use crate::parser::statement::statement;
 use crate::parser::token::Token;
 use crate::parser::token::TokenKind;
 use crate::parser::token::Tokenizer;
-use crate::visit::VisitControl;
-use crate::visit::VisitorMut;
-use crate::visit::WalkMut;
 
 pub fn tokenize_sql(sql: &str) -> Result<Vec<Token<'_>>> {
     Tokenizer::new(sql).collect::<Result<Vec<_>>>()
@@ -242,6 +229,10 @@ fn assert_reparse(sql: &str, stmt: StatementWithFormat) -> std::result::Result<(
 
 #[cfg(debug_assertions)]
 fn reset_ast(mut stmt: StatementWithFormat) -> StatementWithFormat {
+    use crate::visit::VisitControl;
+    use crate::visit::VisitorMut;
+    use crate::visit::WalkMut;
+
     struct ResetAst;
 
     impl ResetAst {

src/query/ast/src/visit.rs

@@ -88,6 +88,8 @@ pub enum VisitControl<B = ()> {
     Break(B),
 }
 
+pub type VisitResult = Result<VisitControl, !>;
+
 /// Pre-order visitor hooks for immutable AST traversal.
 ///
 /// Returning `Continue` lets the walker descend into children automatically.

src/query/sql/src/planner/binder/aggregate.rs

@@ -60,8 +60,8 @@ use crate::plans::GroupingSets;
 use crate::plans::ScalarExpr;
 use crate::plans::ScalarItem;
 use crate::plans::UDAFCall;
-use crate::plans::Visitor;
-use crate::plans::VisitorMut;
+use crate::plans::Visitor as ScalarVisitor;
+use crate::plans::VisitorMut as ScalarVisitorMut;
 use crate::plans::walk_expr_mut;
 
 /// Information for `GROUPING SETS`.
@@ -763,7 +763,7 @@ struct ExistingAggregateRewriter<'a> {
     error_message: &'a str,
 }
 
-impl<'a> VisitorMut<'a> for ExistingAggregateRewriter<'a> {
+impl<'a> ScalarVisitorMut<'a> for ExistingAggregateRewriter<'a> {
     fn visit(&mut self, expr: &'a mut ScalarExpr) -> Result<()> {
         match expr {
             ScalarExpr::AggregateFunction(aggregate) => {
@@ -816,7 +816,7 @@ impl<'a> VisitorMut<'a> for ExistingAggregateRewriter<'a> {
     }
 }
 
-impl<'a> VisitorMut<'a> for AggregateRewriter<'a> {
+impl<'a> ScalarVisitorMut<'a> for AggregateRewriter<'a> {
     fn visit(&mut self, expr: &'a mut ScalarExpr) -> Result<()> {
         match expr {
             ScalarExpr::AggregateFunction(aggregate) => {
@@ -906,8 +906,7 @@ impl Binder {
             }
         }
 
-        let original_context = bind_context.expr_context.clone();
-        bind_context.set_expr_context(ExprContext::GroupClaue);
+        let original_context = bind_context.replace_expr_context(ExprContext::GroupClaue);
 
         let group_by = Self::expand_group(group_by.clone())?;
         match &group_by {
@@ -935,7 +934,7 @@ impl Binder {
             }
             _ => unreachable!(),
         }
-        bind_context.set_expr_context(original_context);
+        bind_context.expr_context = original_context;
         Ok(())
     }
 
@@ -1472,6 +1471,34 @@ impl Binder {
             Ok((scalar.clone(), scalar.data_type()?))
         }
     }
+
+    pub(super) fn bind_and_rewrite_aggregate_expr(
+        &mut self,
+        bind_context: &mut BindContext,
+        aliases: &[(String, ScalarExpr)],
+        expr_context: ExprContext,
+        expr: &Expr,
+    ) -> Result<ScalarExpr> {
+        let original_context = bind_context.replace_expr_context(expr_context);
+
+        let mut scalar_binder = ScalarBinder::new(
+            bind_context,
+            self.ctx.clone(),
+            &self.name_resolution_ctx,
+            self.metadata.clone(),
+            aliases,
+        );
+
+        let (mut result, _) = scalar_binder.bind(expr)?;
+        AggregateRewriter::rewrite_expr(
+            &mut bind_context.aggregate_info,
+            self.metadata.clone(),
+            &mut result,
+        )?;
+
+        bind_context.expr_context = original_context;
+        Ok(result)
+    }
 }
 
 fn build_replaced_aggregate_column(