Skip to main content

sql_cli/query_plan/
having_alias_transformer.rs

1//! HAVING clause auto-aliasing transformer
2//!
3//! This transformer automatically adds aliases to aggregate functions in SELECT
4//! clauses and rewrites HAVING clauses to use those aliases instead of the
5//! aggregate function expressions.
6//!
7//! # Problem
8//!
9//! Users often write queries like:
10//! ```sql
11//! SELECT region, COUNT(*) FROM sales GROUP BY region HAVING COUNT(*) > 5
12//! ```
13//!
14//! This fails because the executor can't evaluate `COUNT(*)` in the HAVING
15//! clause - it needs a column reference.
16//!
17//! # Solution
18//!
19//! The transformer rewrites to:
20//! ```sql
21//! SELECT region, COUNT(*) as __agg_1 FROM sales GROUP BY region HAVING __agg_1 > 5
22//! ```
23//!
24//! # Algorithm
25//!
26//! 1. Find all aggregate functions in SELECT clause
27//! 2. For each aggregate without an explicit alias, generate one (__agg_N)
28//! 3. Scan HAVING clause for matching aggregate expressions
29//! 4. Replace aggregate expressions with column references to the aliases
30
31use crate::query_plan::pipeline::ASTTransformer;
32use crate::sql::parser::ast::{
33    CTEType, ColumnRef, QuoteStyle, SelectItem, SelectStatement, SqlExpression, TableSource,
34};
35use crate::sql::parser::walk;
36use anyhow::Result;
37use std::collections::HashMap;
38use tracing::debug;
39
40/// Prefix used for aggregates promoted from HAVING into SELECT.
41/// Columns with this prefix are hidden from the final output.
42pub const HIDDEN_AGG_PREFIX: &str = "__hidden_agg_";
43
44/// Transformer that adds aliases to aggregates and rewrites HAVING clauses
45pub struct HavingAliasTransformer {
46    /// Counter for generating unique alias names
47    alias_counter: usize,
48    /// Counter for HAVING-promoted (hidden) aggregates
49    hidden_counter: usize,
50}
51
52impl HavingAliasTransformer {
53    pub fn new() -> Self {
54        Self {
55            alias_counter: 0,
56            hidden_counter: 0,
57        }
58    }
59
60    /// Check if an expression is an aggregate function
61    fn is_aggregate_function(expr: &SqlExpression) -> bool {
62        matches!(
63            expr,
64            SqlExpression::FunctionCall { name, .. }
65                if matches!(
66                    name.to_uppercase().as_str(),
67                    "COUNT" | "SUM" | "AVG" | "MIN" | "MAX" | "COUNT_DISTINCT"
68                )
69        )
70    }
71
72    /// Generate a unique alias name
73    fn generate_alias(&mut self) -> String {
74        self.alias_counter += 1;
75        format!("__agg_{}", self.alias_counter)
76    }
77
78    /// Normalize an aggregate expression to a canonical form for comparison
79    fn normalize_aggregate_expr(expr: &SqlExpression) -> String {
80        match expr {
81            SqlExpression::FunctionCall { name, args, .. } => {
82                let args_str = args
83                    .iter()
84                    .map(|arg| match arg {
85                        SqlExpression::Column(col_ref) => {
86                            format!("{}", col_ref.name)
87                        }
88                        SqlExpression::StringLiteral(s) => format!("'{}'", s),
89                        SqlExpression::NumberLiteral(n) => n.clone(),
90                        _ => format!("{:?}", arg), // Fallback for complex args
91                    })
92                    .collect::<Vec<_>>()
93                    .join(",");
94                format!("{}({})", name.to_uppercase(), args_str)
95            }
96            _ => format!("{:?}", expr),
97        }
98    }
99
100    /// Extract aggregate functions from SELECT clause and ensure they have aliases
101    fn ensure_aggregate_aliases(
102        &mut self,
103        select_items: &mut Vec<SelectItem>,
104    ) -> HashMap<String, String> {
105        let mut aggregate_map = HashMap::new();
106
107        for item in select_items.iter_mut() {
108            if let SelectItem::Expression { expr, alias, .. } = item {
109                if Self::is_aggregate_function(expr) {
110                    // Generate alias if none exists
111                    if alias.is_empty() {
112                        *alias = self.generate_alias();
113                        debug!(
114                            "Generated alias '{}' for aggregate: {}",
115                            alias,
116                            Self::normalize_aggregate_expr(expr)
117                        );
118                    }
119
120                    // Map normalized expression to alias
121                    let normalized = Self::normalize_aggregate_expr(expr);
122                    aggregate_map.insert(normalized, alias.clone());
123                }
124            }
125        }
126
127        aggregate_map
128    }
129
130    /// Generate a unique hidden alias name for aggregates promoted from HAVING
131    fn generate_hidden_alias(&mut self) -> String {
132        self.hidden_counter += 1;
133        format!("{}{}", HIDDEN_AGG_PREFIX, self.hidden_counter)
134    }
135
136    /// Collect all aggregate function calls from a HAVING expression
137    ///
138    /// The one real rule is the aggregate itself; everything else is plain
139    /// traversal. This deliberately does **not** delegate the aggregate arm to
140    /// the walker: recursing into an aggregate's arguments would break the
141    /// "no nested aggregates" invariant the old code kept by hand.
142    ///
143    /// The old version matched only `BinaryOp`, `Not` and `FunctionCall` before
144    /// its catch-all, so an aggregate reached through `BETWEEN`, `IN` or `CASE`
145    /// was never collected and never promoted -- P9, silently wrong rows.
146    fn collect_aggregates_in_having(expr: &SqlExpression, found: &mut Vec<SqlExpression>) {
147        if Self::is_aggregate_function(expr) {
148            found.push(expr.clone());
149            return;
150        }
151        walk::visit_children(expr, |child| {
152            Self::collect_aggregates_in_having(child, found)
153        });
154    }
155
156    /// Promote aggregates in HAVING that aren't already in SELECT into hidden
157    /// SELECT items. Returns updated aggregate_map with new entries.
158    fn promote_having_aggregates(
159        &mut self,
160        having_expr: &SqlExpression,
161        select_items: &mut Vec<SelectItem>,
162        aggregate_map: &mut HashMap<String, String>,
163    ) {
164        let mut having_aggs = Vec::new();
165        Self::collect_aggregates_in_having(having_expr, &mut having_aggs);
166
167        for agg in having_aggs {
168            let normalized = Self::normalize_aggregate_expr(&agg);
169            if aggregate_map.contains_key(&normalized) {
170                continue; // Already in SELECT
171            }
172
173            let hidden_alias = self.generate_hidden_alias();
174            debug!(
175                "Promoting HAVING aggregate {} as hidden SELECT item '{}'",
176                normalized, hidden_alias
177            );
178
179            select_items.push(SelectItem::Expression {
180                expr: agg,
181                alias: hidden_alias.clone(),
182                leading_comments: Vec::new(),
183                trailing_comment: None,
184            });
185
186            aggregate_map.insert(normalized, hidden_alias);
187        }
188    }
189
190    /// Rewrite a HAVING expression to use aliases instead of aggregates
191    ///
192    /// Mirrors [`Self::collect_aggregates_in_having`]: handle the aggregate,
193    /// delegate the rest, and do not descend into an aggregate's arguments.
194    ///
195    /// Note the subquery boundary works in our favour here — `map_children`
196    /// does not descend into a nested `SelectStatement`, so an aggregate
197    /// belonging to a subquery's own scope is correctly left alone.
198    fn rewrite_having_expression(
199        expr: SqlExpression,
200        aggregate_map: &HashMap<String, String>,
201    ) -> SqlExpression {
202        if Self::is_aggregate_function(&expr) {
203            let normalized = Self::normalize_aggregate_expr(&expr);
204            return match aggregate_map.get(&normalized) {
205                Some(alias) => {
206                    debug!(
207                        "Rewriting aggregate {} to column reference {}",
208                        normalized, alias
209                    );
210                    SqlExpression::Column(ColumnRef {
211                        name: alias.clone(),
212                        quote_style: QuoteStyle::None,
213                        table_prefix: None,
214                    })
215                }
216                // Aggregate not found in SELECT - leave as is
217                None => expr,
218            };
219        }
220
221        walk::map_children(expr, |child| {
222            Self::rewrite_having_expression(child, aggregate_map)
223        })
224    }
225
226    /// Transform a SelectStatement and recursively apply to nested statements
227    /// (CTEs, FROM subqueries, set operations). This ensures HAVING clauses in
228    /// any nested query — including subqueries in FROM — are properly rewritten.
229    #[allow(deprecated)]
230    fn transform_statement(&mut self, mut stmt: SelectStatement) -> Result<SelectStatement> {
231        // Step A: recurse into CTEs
232        for cte in stmt.ctes.iter_mut() {
233            if let CTEType::Standard(ref mut inner) = cte.cte_type {
234                let taken = std::mem::take(inner);
235                *inner = self.transform_statement(taken)?;
236            }
237        }
238
239        // Step B: recurse into FROM source (DerivedTable)
240        if let Some(TableSource::DerivedTable { query, .. }) = stmt.from_source.as_mut() {
241            let taken = std::mem::take(query.as_mut());
242            **query = self.transform_statement(taken)?;
243        }
244
245        // Step B2: recurse into legacy from_subquery
246        if let Some(subq) = stmt.from_subquery.as_mut() {
247            let taken = std::mem::take(subq.as_mut());
248            **subq = self.transform_statement(taken)?;
249        }
250
251        // Step C: recurse into set operation right-hand sides
252        for (_op, rhs) in stmt.set_operations.iter_mut() {
253            let taken = std::mem::take(rhs.as_mut());
254            **rhs = self.transform_statement(taken)?;
255        }
256
257        // Step D: apply HAVING transformation at this level
258        self.apply_having_rewrite(&mut stmt);
259
260        Ok(stmt)
261    }
262
263    /// Apply HAVING aggregate promotion and rewriting to a single statement
264    /// (no recursion — the caller is responsible for that).
265    fn apply_having_rewrite(&mut self, stmt: &mut SelectStatement) {
266        // Only process if there's a HAVING clause
267        if stmt.having.is_none() {
268            return;
269        }
270
271        // Step 1: Ensure all aggregates in SELECT have aliases and build mapping
272        let mut aggregate_map = self.ensure_aggregate_aliases(&mut stmt.select_items);
273
274        // Step 1b: Promote any HAVING-only aggregates into SELECT with hidden aliases
275        if let Some(ref having_expr) = stmt.having {
276            self.promote_having_aggregates(having_expr, &mut stmt.select_items, &mut aggregate_map);
277        }
278
279        if aggregate_map.is_empty() {
280            return;
281        }
282
283        // Step 2: Rewrite HAVING clause to use aliases
284        if let Some(having_expr) = stmt.having.take() {
285            // Snapshot for the log line only, so the rewrite can consume the
286            // expression rather than deep-cloning it at every level.
287            let before = format!("{having_expr:?}");
288            let rewritten = Self::rewrite_having_expression(having_expr, &aggregate_map);
289            if before != format!("{rewritten:?}") {
290                debug!(
291                    "Rewrote HAVING clause with {} aggregate alias(es)",
292                    aggregate_map.len()
293                );
294            }
295            stmt.having = Some(rewritten);
296        }
297    }
298}
299
300impl Default for HavingAliasTransformer {
301    fn default() -> Self {
302        Self::new()
303    }
304}
305
306impl ASTTransformer for HavingAliasTransformer {
307    fn name(&self) -> &str {
308        "HavingAliasTransformer"
309    }
310
311    fn description(&self) -> &str {
312        "Adds aliases to aggregate functions and rewrites HAVING clauses to use them"
313    }
314
315    fn transform(&mut self, stmt: SelectStatement) -> Result<SelectStatement> {
316        self.transform_statement(stmt)
317    }
318
319    fn begin(&mut self) -> Result<()> {
320        // Reset counters for each query
321        self.alias_counter = 0;
322        self.hidden_counter = 0;
323        Ok(())
324    }
325}
326
327#[cfg(test)]
328mod tests {
329    use super::*;
330
331    #[test]
332    fn test_is_aggregate_function() {
333        let count_expr = SqlExpression::FunctionCall {
334            name: "COUNT".to_string(),
335            args: vec![SqlExpression::Column(ColumnRef {
336                name: "*".to_string(),
337                quote_style: QuoteStyle::None,
338                table_prefix: None,
339            })],
340            distinct: false,
341        };
342        assert!(HavingAliasTransformer::is_aggregate_function(&count_expr));
343
344        let sum_expr = SqlExpression::FunctionCall {
345            name: "SUM".to_string(),
346            args: vec![SqlExpression::Column(ColumnRef {
347                name: "amount".to_string(),
348                quote_style: QuoteStyle::None,
349                table_prefix: None,
350            })],
351            distinct: false,
352        };
353        assert!(HavingAliasTransformer::is_aggregate_function(&sum_expr));
354
355        let non_agg = SqlExpression::FunctionCall {
356            name: "UPPER".to_string(),
357            args: vec![],
358            distinct: false,
359        };
360        assert!(!HavingAliasTransformer::is_aggregate_function(&non_agg));
361    }
362
363    #[test]
364    fn test_normalize_aggregate_expr() {
365        let count_star = SqlExpression::FunctionCall {
366            name: "count".to_string(),
367            args: vec![SqlExpression::Column(ColumnRef {
368                name: "*".to_string(),
369                quote_style: QuoteStyle::None,
370                table_prefix: None,
371            })],
372            distinct: false,
373        };
374        assert_eq!(
375            HavingAliasTransformer::normalize_aggregate_expr(&count_star),
376            "COUNT(*)"
377        );
378
379        let sum_amount = SqlExpression::FunctionCall {
380            name: "SUM".to_string(),
381            args: vec![SqlExpression::Column(ColumnRef {
382                name: "amount".to_string(),
383                quote_style: QuoteStyle::None,
384                table_prefix: None,
385            })],
386            distinct: false,
387        };
388        assert_eq!(
389            HavingAliasTransformer::normalize_aggregate_expr(&sum_amount),
390            "SUM(amount)"
391        );
392    }
393
394    #[test]
395    fn test_generate_alias() {
396        let mut transformer = HavingAliasTransformer::new();
397        assert_eq!(transformer.generate_alias(), "__agg_1");
398        assert_eq!(transformer.generate_alias(), "__agg_2");
399        assert_eq!(transformer.generate_alias(), "__agg_3");
400    }
401
402    /// P9 regression. An aggregate reached through `BETWEEN` / `IN` / `CASE`
403    /// used to fall into the catch-all: never collected, never promoted, never
404    /// rewritten — so the predicate silently didn't filter. The corpus pins the
405    /// row counts against DuckDB; this pins the mechanism, so a regression
406    /// shows up in `cargo test` and not only in the parity harness.
407    #[test]
408    fn rewrites_aggregates_nested_in_non_comparison_operators() {
409        use crate::sql::recursive_parser::Parser;
410
411        for (label, sql) in [
412            (
413                "BETWEEN",
414                "SELECT region, COUNT(*) AS n FROM t GROUP BY region HAVING COUNT(*) BETWEEN 1 AND 2",
415            ),
416            (
417                "IN",
418                "SELECT region, COUNT(*) AS n FROM t GROUP BY region HAVING COUNT(*) IN (4, 5)",
419            ),
420            (
421                "CASE",
422                "SELECT region, COUNT(*) AS n FROM t GROUP BY region HAVING CASE WHEN COUNT(*) > 2 THEN 1 ELSE 0 END = 1",
423            ),
424        ] {
425            let stmt = Parser::new(sql).parse().expect("should parse");
426            let result = HavingAliasTransformer::new()
427                .transform(stmt)
428                .expect("transform should succeed");
429
430            let having = result.having.expect("having clause");
431            let mut aggregates_left = 0;
432            crate::sql::parser::walk::visit_all(&having, &mut |e| {
433                if HavingAliasTransformer::is_aggregate_function(e) {
434                    aggregates_left += 1;
435                }
436            });
437
438            assert_eq!(
439                aggregates_left, 0,
440                "{label}: every aggregate in HAVING must be rewritten to its alias, \
441                 leaving none behind; got {having:?}"
442            );
443        }
444    }
445
446    /// The other half of the invariant: an aggregate's *arguments* must stay
447    /// untraversed, which is why the aggregate arm returns early instead of
448    /// delegating to the walker.
449    #[test]
450    fn does_not_descend_into_aggregate_arguments() {
451        let inner = SqlExpression::FunctionCall {
452            name: "COUNT".to_string(),
453            args: vec![SqlExpression::Column(ColumnRef {
454                name: "x".to_string(),
455                quote_style: QuoteStyle::None,
456                table_prefix: None,
457            })],
458            distinct: false,
459        };
460
461        let mut found = Vec::new();
462        HavingAliasTransformer::collect_aggregates_in_having(&inner, &mut found);
463        assert_eq!(
464            found.len(),
465            1,
466            "an aggregate is collected as one unit, not walked into"
467        );
468    }
469
470    #[test]
471    fn test_transform_with_no_having() {
472        let mut transformer = HavingAliasTransformer::new();
473        let stmt = SelectStatement {
474            having: None,
475            ..Default::default()
476        };
477
478        let result = transformer.transform(stmt);
479        assert!(result.is_ok());
480    }
481
482    #[test]
483    fn test_transform_adds_alias_and_rewrites_having() {
484        let mut transformer = HavingAliasTransformer::new();
485
486        let count_expr = SqlExpression::FunctionCall {
487            name: "COUNT".to_string(),
488            args: vec![SqlExpression::Column(ColumnRef {
489                name: "*".to_string(),
490                quote_style: QuoteStyle::None,
491                table_prefix: None,
492            })],
493            distinct: false,
494        };
495
496        let stmt = SelectStatement {
497            select_items: vec![SelectItem::Expression {
498                expr: count_expr.clone(),
499                alias: String::new(), // No alias initially
500                leading_comments: Vec::new(),
501                trailing_comment: None,
502            }],
503            having: Some(SqlExpression::BinaryOp {
504                left: Box::new(count_expr.clone()),
505                op: ">".to_string(),
506                right: Box::new(SqlExpression::NumberLiteral("5".to_string())),
507            }),
508            ..Default::default()
509        };
510
511        let result = transformer.transform(stmt).unwrap();
512
513        // Check that alias was added to SELECT
514        if let SelectItem::Expression { alias, .. } = &result.select_items[0] {
515            assert_eq!(alias, "__agg_1");
516        } else {
517            panic!("Expected Expression select item");
518        }
519
520        // Check that HAVING was rewritten to use alias
521        if let Some(SqlExpression::BinaryOp { left, .. }) = &result.having {
522            match left.as_ref() {
523                SqlExpression::Column(col_ref) => {
524                    assert_eq!(col_ref.name, "__agg_1");
525                }
526                _ => panic!("Expected column reference in HAVING, got: {:?}", left),
527            }
528        } else {
529            panic!("Expected BinaryOp in HAVING");
530        }
531    }
532}