fix

forsaken628 · forsaken628 · commit a6ceca131ce3 · 2026-04-01T20:52:44.000+08:00
diff --git a/.gitignore b/.gitignore
@@ -90,6 +90,7 @@ benchmark/clickbench/results
 .lycheecache
 
 .claude
+.codex
 
 # tmp
 tmp
diff --git a/src/query/sql/src/planner/binder/aggregate.rs b/src/query/sql/src/planner/binder/aggregate.rs
@@ -891,8 +891,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 {
@@ -920,7 +919,7 @@ impl Binder {
             }
             _ => unreachable!(),
         }
-        bind_context.set_expr_context(original_context);
+        bind_context.expr_context = original_context;
         Ok(())
     }
 
@@ -1448,6 +1447,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(
diff --git a/src/query/sql/src/planner/binder/bind_context.rs b/src/query/sql/src/planner/binder/bind_context.rs
@@ -65,7 +65,7 @@ use crate::plans::ScalarExpr;
 
 /// Context of current expression, this is used to check if
 /// the expression is valid in current context.
-#[derive(Debug, Clone, Default, EnumAsInner)]
+#[derive(Debug, Clone, Copy, Default, EnumAsInner)]
 pub enum ExprContext {
     SelectClause,
     WhereClause,
@@ -919,8 +919,10 @@ impl BindContext {
         self.columns.iter().map(|c| c.index).collect()
     }
 
-    pub fn set_expr_context(&mut self, expr_context: ExprContext) {
-        self.expr_context = expr_context;
+    pub fn replace_expr_context(&mut self, new: ExprContext) -> ExprContext {
+        let old = self.expr_context;
+        self.expr_context = new;
+        old
     }
 }
 
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
@@ -54,6 +54,7 @@ use crate::optimizer::ir::SExpr;
 use crate::planner::QueryExecutor;
 use crate::planner::binder::BindContext;
 use crate::planner::binder::Binder;
+use crate::planner::binder::ExprContext;
 
 impl Binder {
     #[async_backtrace::framed]
@@ -143,6 +144,24 @@ impl Binder {
 
         self.analyze_aggregate_select(&mut from_context, &mut select_list)?;
 
+        if let Some(having) = &stmt.having {
+            self.bind_and_rewrite_aggregate_expr(
+                &mut from_context,
+                &semantic_aliases,
+                ExprContext::HavingClause,
+                having,
+            )?;
+        }
+
+        for order in order_by {
+            self.bind_and_rewrite_aggregate_expr(
+                &mut from_context,
+                &semantic_aliases,
+                ExprContext::OrderByClause,
+                &order.expr,
+            )?;
+        }
+
         // `analyze_window` should behind `analyze_aggregate_select`,
         // because `analyze_window` will rewrite the aggregate functions in the window function's arguments.
         self.analyze_window(&mut from_context, &mut select_list)?;
@@ -247,7 +266,7 @@ impl Binder {
         }
 
         if stmt.distinct {
-            s_expr = self.bind_distinct(stmt.span, &mut from_context, &mut select_info, s_expr)?;
+            s_expr = self.bind_distinct(stmt.span, &mut select_info, s_expr)?;
         }
 
         s_expr = self.bind_projection(&mut from_context, select_info, s_expr)?;
diff --git a/src/query/sql/src/planner/binder/distinct.rs b/src/query/sql/src/planner/binder/distinct.rs
@@ -17,7 +17,6 @@ use std::sync::Arc;
 use databend_common_ast::Span;
 use databend_common_exception::Result;
 
-use crate::BindContext;
 use crate::binder::Binder;
 use crate::binder::project::SelectInfo;
 use crate::optimizer::ir::SExpr;
@@ -28,7 +27,6 @@ impl Binder {
     pub fn bind_distinct(
         &self,
         span: Span,
-        _bind_context: &mut BindContext,
         select_info: &mut SelectInfo,
         child: SExpr,
     ) -> Result<SExpr> {
diff --git a/src/query/sql/src/planner/binder/having.rs b/src/query/sql/src/planner/binder/having.rs
@@ -22,8 +22,6 @@ use super::Finder;
 use crate::BindContext;
 use crate::Binder;
 use crate::binder::ExprContext;
-use crate::binder::ScalarBinder;
-use crate::binder::aggregate::AggregateRewriter;
 use crate::binder::split_conjunctions;
 use crate::optimizer::ir::SExpr;
 use crate::planner::semantic::GroupingChecker;
@@ -41,22 +39,12 @@ impl Binder {
         aliases: &[(String, ScalarExpr)],
         having: &Expr,
     ) -> Result<ScalarExpr> {
-        bind_context.set_expr_context(ExprContext::HavingClause);
-
-        let mut scalar_binder = ScalarBinder::new(
+        self.bind_and_rewrite_aggregate_expr(
             bind_context,
-            self.ctx.clone(),
-            &self.name_resolution_ctx,
-            self.metadata.clone(),
             aliases,
-        );
-        let (mut scalar, _) = scalar_binder.bind(having)?;
-        AggregateRewriter::rewrite_expr(
-            &mut bind_context.aggregate_info,
-            self.metadata.clone(),
-            &mut scalar,
-        )?;
-        Ok(scalar)
+            ExprContext::HavingClause,
+            having,
+        )
     }
 
     pub fn bind_having(
@@ -65,7 +53,7 @@ impl Binder {
         having: ScalarExpr,
         child: SExpr,
     ) -> Result<SExpr> {
-        bind_context.set_expr_context(ExprContext::HavingClause);
+        bind_context.expr_context = ExprContext::HavingClause;
 
         let f = |scalar: &ScalarExpr| matches!(scalar, ScalarExpr::WindowFunction(_));
         let mut finder = Finder::new(&f);
diff --git a/src/query/sql/src/planner/binder/project.rs b/src/query/sql/src/planner/binder/project.rs
@@ -325,7 +325,7 @@ impl Binder {
         select_info: SelectInfo,
         child: SExpr,
     ) -> Result<SExpr> {
-        bind_context.set_expr_context(ExprContext::SelectClause);
+        bind_context.expr_context = ExprContext::SelectClause;
         let plan = select_info.into_projection_plan()?;
         let eval_scalar = EvalScalar { items: plan.items };
         let new_expr = SExpr::create_unary(Arc::new(eval_scalar.into()), Arc::new(child));
@@ -354,7 +354,7 @@ impl Binder {
         input_context: &mut BindContext,
         select_list: &'a [SelectTarget],
     ) -> Result<SelectList<'a>> {
-        input_context.set_expr_context(ExprContext::SelectClause);
+        input_context.expr_context = ExprContext::SelectClause;
 
         let mut output = SelectList::default();
         let mut prev_aliases = Vec::new();
diff --git a/src/query/sql/src/planner/binder/qualify.rs b/src/query/sql/src/planner/binder/qualify.rs
@@ -43,7 +43,7 @@ impl Binder {
         aliases: &[(String, ScalarExpr)],
         qualify: &Expr,
     ) -> Result<ScalarExpr> {
-        bind_context.set_expr_context(ExprContext::QualifyClause);
+        bind_context.expr_context = ExprContext::QualifyClause;
         let mut scalar_binder = ScalarBinder::new(
             bind_context,
             self.ctx.clone(),
@@ -63,7 +63,7 @@ impl Binder {
         qualify: ScalarExpr,
         child: SExpr,
     ) -> Result<SExpr> {
-        bind_context.set_expr_context(ExprContext::QualifyClause);
+        bind_context.expr_context = ExprContext::QualifyClause;
 
         let scalar = {
             let mut qualify = qualify;
diff --git a/src/query/sql/src/planner/binder/select.rs b/src/query/sql/src/planner/binder/select.rs
@@ -75,8 +75,7 @@ impl Binder {
         expr: &Expr,
         child: SExpr,
     ) -> Result<(SExpr, ScalarExpr)> {
-        let last_expr_context = bind_context.expr_context.clone();
-        bind_context.set_expr_context(ExprContext::WhereClause);
+        let last_expr_context = bind_context.replace_expr_context(ExprContext::WhereClause);
 
         let mut scalar_binder = ScalarBinder::new(
             bind_context,
@@ -103,7 +102,7 @@ impl Binder {
             predicates: split_conjunctions(&scalar),
         };
         let new_expr = SExpr::create_unary(Arc::new(filter_plan.into()), Arc::new(child));
-        bind_context.set_expr_context(last_expr_context);
+        bind_context.expr_context = last_expr_context;
 
         Ok((new_expr, scalar))
     }
@@ -308,8 +307,7 @@ impl Binder {
         if distinct {
             let mut select_info =
                 SelectInfo::from_columns(new_bind_context.all_column_bindings().to_vec());
-            new_expr =
-                self.bind_distinct(left_span, &mut new_bind_context, &mut select_info, new_expr)?;
+            new_expr = self.bind_distinct(left_span, &mut select_info, new_expr)?;
         }
 
         Ok((new_expr, new_bind_context))
@@ -381,7 +379,7 @@ impl Binder {
 
         // then apply distinct
         let mut select_info = SelectInfo::from_columns(left_context.all_column_bindings().to_vec());
-        let s_expr = self.bind_distinct(left_span, &mut left_context, &mut select_info, s_expr)?;
+        let s_expr = self.bind_distinct(left_span, &mut select_info, s_expr)?;
         Ok((s_expr, left_context))
     }
 
diff --git a/src/query/sql/src/planner/binder/sort.rs b/src/query/sql/src/planner/binder/sort.rs
@@ -62,7 +62,7 @@ impl Binder {
         order_by: &[OrderByExpr],
         distinct: bool,
     ) -> Result<OrderItems> {
-        bind_context.set_expr_context(ExprContext::OrderByClause);
+        bind_context.expr_context = ExprContext::OrderByClause;
         let settings = self.ctx.get_settings();
         let default_nulls_first = settings.get_nulls_first();
 
diff --git a/src/query/sql/src/planner/dataframe.rs b/src/query/sql/src/planner/dataframe.rs
@@ -289,12 +289,9 @@ impl Dataframe {
         let mut select_info = self
             .binder
             .analyze_projection(&self.bind_context, &select_list)?;
-        self.s_expr = self.binder.bind_distinct(
-            None,
-            &mut self.bind_context,
-            &mut select_info,
-            self.s_expr,
-        )?;
+        self.s_expr = self
+            .binder
+            .bind_distinct(None, &mut select_info, self.s_expr)?;
         self.apply_select_output(select_info)
     }
 
diff --git a/src/query/sql/src/planner/expression/expression_parser.rs b/src/query/sql/src/planner/expression/expression_parser.rs
@@ -349,7 +349,7 @@ pub fn parse_lambda_expr(
     // Use parent metadata if provided (for masking policies on outer columns)
     // Otherwise create empty metadata (for better performance in community edition)
     let metadata = parent_metadata.unwrap_or_else(|| Arc::new(RwLock::new(Metadata::default())));
-    lambda_context.set_expr_context(ExprContext::InLambdaFunction);
+    lambda_context.expr_context = ExprContext::InLambdaFunction;
 
     for (lambda_column, lambda_column_type) in lambda_columns.iter() {
         let column_index = lambda_context.next_column_index();
diff --git a/src/query/sql/src/planner/semantic/type_check.rs b/src/query/sql/src/planner/semantic/type_check.rs
@@ -2236,11 +2236,10 @@ impl<'a> TypeChecker<'a> {
         // column-first fallback so `sum(c1)` can bind a same-select alias when
         // there is no real `c1` column.
         self.in_aggregate_function = true;
-        let original_context = self.bind_context.expr_context.clone();
+        let original_context = self.bind_context.expr_context;
         let disallow_alias_resolution = original_context.prefer_resolve_alias();
         if disallow_alias_resolution {
-            self.bind_context
-                .set_expr_context(ExprContext::InAggregateFunction);
+            self.bind_context.expr_context = ExprContext::InAggregateFunction;
         }
         let arguments_result = (|| {
             let mut arguments = vec![];
@@ -2253,7 +2252,7 @@ impl<'a> TypeChecker<'a> {
             Ok::<_, ErrorCode>((arguments, arg_types))
         })();
         if disallow_alias_resolution {
-            self.bind_context.set_expr_context(original_context.clone());
+            self.bind_context.expr_context = original_context;
         }
         self.in_aggregate_function = false;
         let (mut arguments, mut arg_types) = arguments_result?;
@@ -2267,12 +2266,11 @@ impl<'a> TypeChecker<'a> {
                      nulls_first,
                  }| {
                     if disallow_alias_resolution {
-                        self.bind_context
-                            .set_expr_context(ExprContext::InAggregateFunction);
+                        self.bind_context.expr_context = ExprContext::InAggregateFunction;
                     }
                     let result = self.resolve(expr);
                     if disallow_alias_resolution {
-                        self.bind_context.set_expr_context(original_context.clone());
+                        self.bind_context.expr_context = original_context;
                     }
                     let box (scalar_expr, _) = result?;
 
@@ -3288,9 +3286,9 @@ impl<'a> TypeChecker<'a> {
             .set_span(span));
         }
 
-        let original_context = self.bind_context.expr_context.clone();
-        self.bind_context
-            .set_expr_context(ExprContext::InSetReturningFunction);
+        let original_context = self
+            .bind_context
+            .replace_expr_context(ExprContext::InSetReturningFunction);
 
         let mut arguments = Vec::with_capacity(args.len());
         for arg in args.iter() {
@@ -3299,7 +3297,7 @@ impl<'a> TypeChecker<'a> {
         }
 
         // Restore the original context
-        self.bind_context.set_expr_context(original_context);
+        self.bind_context.expr_context = original_context;
 
         let srf_scalar = ScalarExpr::FunctionCall(FunctionCall {
             span,
@@ -6477,9 +6475,9 @@ impl<'a> TypeChecker<'a> {
                     .set_span(span),
             );
         }
-        let original_context = self.bind_context.expr_context.clone();
-        self.bind_context
-            .set_expr_context(ExprContext::InAsyncFunction);
+        let original_context = self
+            .bind_context
+            .replace_expr_context(ExprContext::InAsyncFunction);
         let result = match func_name {
             "nextval" => self.resolve_nextval_async_function(span, func_name, arguments)?,
             "dict_get" => self.resolve_dict_get_async_function(span, func_name, arguments)?,
@@ -6493,7 +6491,7 @@ impl<'a> TypeChecker<'a> {
             }
         };
         // Restore the original context
-        self.bind_context.set_expr_context(original_context);
+        self.bind_context.expr_context = original_context;
         self.bind_context.have_async_func = true;
         Ok(result)
     }
diff --git a/src/query/sql/tests/it/semantic/binder.rs b/src/query/sql/tests/it/semantic/binder.rs
@@ -185,6 +185,18 @@ async fn test_binder_with_lite_table_context() -> Result<()> {
             setup_sqls: &["CREATE TABLE t(number UInt64)"],
             sql: "SELECT row_number() OVER (ORDER BY sum(number)) FROM t",
         },
+        SqlTestCase {
+            name: "window_order_reuses_having_aggregate",
+            description: "A window ORDER BY clause should be able to reuse an aggregate introduced later by HAVING.",
+            setup_sqls: &["CREATE TABLE t(number UInt64)"],
+            sql: "SELECT row_number() OVER (ORDER BY sum(number)) FROM t HAVING sum(number) > 0",
+        },
+        SqlTestCase {
+            name: "window_order_reuses_order_by_aggregate",
+            description: "A window ORDER BY clause should be able to reuse an aggregate introduced later by ORDER BY.",
+            setup_sqls: &["CREATE TABLE t(number UInt64)"],
+            sql: "SELECT row_number() OVER (ORDER BY sum(number)) FROM t ORDER BY sum(number)",
+        },
         SqlTestCase {
             name: "window_order_rejects_window_alias_expansion",
             description: "A window ORDER BY clause must still reject aliases that expand to a prior window expression.",
diff --git a/src/query/sql/tests/it/semantic/binder.txt b/src/query/sql/tests/it/semantic/binder.txt
diff --git a/tests/sqllogictests/suites/mode/standalone/explain/index/explain_inverted_index.test b/tests/sqllogictests/suites/mode/standalone/explain/index/explain_inverted_index.test
diff --git a/tests/sqllogictests/suites/tpch/join_order.test b/tests/sqllogictests/suites/tpch/join_order.test

-Original file line number
+Diff line change
 .lycheecache
 .claude
 +.codex
 # tmp
 tmp
Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ use crate::plans::ScalarExpr;`
`65`	`65`
`66`	`66`	`/// Context of current expression, this is used to check if`
`67`	`67`	`/// the expression is valid in current context.`
`68`		`-#[derive(Debug, Clone, Default, EnumAsInner)]`
	`68`	`+#[derive(Debug, Clone, Copy, Default, EnumAsInner)]`
`69`	`69`	`pub enum ExprContext {`
`70`	`70`	`SelectClause,`
`71`	`71`	`WhereClause,`
`@@ -919,8 +919,10 @@ impl BindContext {`
`919`	`919`	`self.columns.iter().map(\|c\| c.index).collect()`
`920`	`920`	`}`
`921`	`921`
`922`		`- pub fn set_expr_context(&mut self, expr_context: ExprContext) {`
`923`		`- self.expr_context = expr_context;`
	`922`	`+ pub fn replace_expr_context(&mut self, new: ExprContext) -> ExprContext {`
	`923`	`+ let old = self.expr_context;`
	`924`	`+ self.expr_context = new;`
	`925`	`+ old`
`924`	`926`	`}`
`925`	`927`	`}`
`926`	`928`