feat(analyzer): Add support for SELECT alias references in HAVING clause

[mehradpk]

Description

This change enables the HAVING clause to reference SELECT output aliases.

Previously, HAVING expressions were analyzed without resolving SELECT output aliases, which caused queries referencing projected aliases to fail semantic analysis.

This change rewrites the HAVING predicate by using existing OrderByExpressionRewriter prior to expression analysis, aligning alias resolution semantics with existing ORDER BY behavior.

Example:

Before (this query would fail with a semantic analysis error):

HAVING alias 'total' unresolved

SELECT sum(a) AS total
FROM t1
GROUP BY b
HAVING total > 10;
-- Error: Column 'total' cannot be resolved

After (this query works correctly)

HAVING can reference SELECT alias 'total'

SELECT sum(a) AS total
FROM t1
GROUP BY b
HAVING total > 10;

Motivation and Context

Queries that reference SELECT output aliases in the HAVING clause currently fail during analysis due to unresolved aliases. This behavior is inconsistent with ORDER BY, which supports alias resolution.

This change improves SQL compatibility.

Impact

HAVING clause can now reference SELECT output aliases.

No changes to public APIs.
No expected performance impact.
No changes to planning or execution logic.

Test Plan

Updated test case which now also includes negative case. Local testing performed.

Contributor checklist

• Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
• PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
• Documented new properties (with its default value), SQL syntax, functions, or other functionality.
• If release notes are required, they follow the release notes guidelines.
• Adequate tests were added if applicable.
• CI passed.
• If adding new dependencies, verified they have an OpenSSF Scorecard score of 5.0 or higher (or obtained explicit TSC approval for lower scores).

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* Add support for SELECT alias references in HAVING clause

Summary by Sourcery

Enable semantic analysis of HAVING clauses that reference SELECT output aliases by reusing the existing alias-rewriting logic shared with ORDER BY.

New Features:

• Allow HAVING clauses to reference SELECT output aliases for improved SQL compatibility.

Enhancements:

• Generalize the ORDER BY expression rewriter to be parameterized by clause name and reuse it for HAVING alias resolution, including clearer ambiguity error messages.
• Include rewritten HAVING expressions in grouping and aggregation analysis to keep semantics consistent.

Tests:

• Expand analyzer tests to cover successful HAVING references to SELECT aliases across grouped and aggregated queries.

[sourcery-ai]

Reviewer's Guide

Adds support for resolving SELECT output aliases in HAVING clause by reusing the existing OrderByExpressionRewriter, ensuring HAVING predicates are rewritten and analyzed consistently with ORDER BY, and updates tests accordingly.

Sequence diagram for HAVING clause analysis with SELECT alias resolution

sequenceDiagram
    actor Client
    participant StatementAnalyzer
    participant Analysis
    participant OrderByExpressionRewriter

    Client->>StatementAnalyzer: analyze QuerySpecification
    activate StatementAnalyzer

    Note over StatementAnalyzer: analyzeHaving(node, scope)
    StatementAnalyzer->>StatementAnalyzer: extractNamedOutputExpressions(select)
    StatementAnalyzer->>OrderByExpressionRewriter: new OrderByExpressionRewriter(namedOutputExpressions, HAVING)
    activate OrderByExpressionRewriter
    StatementAnalyzer->>OrderByExpressionRewriter: rewrite(predicate)
    OrderByExpressionRewriter-->>StatementAnalyzer: rewrittenPredicate
    deactivate OrderByExpressionRewriter

    StatementAnalyzer->>StatementAnalyzer: analyzeExpression(rewrittenPredicate, scope)
    StatementAnalyzer->>Analysis: recordSubqueries(node, expressionAnalysis)
    StatementAnalyzer->>StatementAnalyzer: getType(rewrittenPredicate)
    StatementAnalyzer->>Analysis: setHaving(node, rewrittenPredicate)

    Note over StatementAnalyzer: visitQuerySpecification(node, scope)
    StatementAnalyzer->>Analysis: getHaving(node)
    Analysis-->>StatementAnalyzer: havingExpression
    alt havingExpression not null
        StatementAnalyzer->>StatementAnalyzer: add havingExpression to sourceExpressions
    else
        StatementAnalyzer->>StatementAnalyzer: add original node.getHaving() to sourceExpressions
    end

    StatementAnalyzer->>StatementAnalyzer: analyzeGroupingOperations(node, sourceExpressions, orderByExpressions)
    StatementAnalyzer->>StatementAnalyzer: analyzeAggregations(node, sourceExpressions, orderByExpressions)
    deactivate StatementAnalyzer

Loading

Updated class diagram for StatementAnalyzer HAVING alias resolution

classDiagram
    class StatementAnalyzer {
        +visitQuerySpecification(node, scope) Scope
        -analyzeHaving(node, scope) void
    }

    class Analysis {
        +setHaving(node, expression) void
        +getHaving(node) Expression
        +setOrderByExpressions(node, expressions) void
        +setOrderByExpressionsFrom(node, sourceExpressions) void
        +recordSubqueries(node, expressionAnalysis) void
    }

    class OrderByExpressionRewriter {
        -assignments Multimap~QualifiedName,Expression~
        -clauseName String
        +OrderByExpressionRewriter(assignments)
        +OrderByExpressionRewriter(assignments, clauseName)
        +rewriteIdentifier(reference, context, treeRewriter) Expression
    }

    class ExpressionAnalysis {
        +getType(expression) Type
        +getWindowFunctions() Collection~FunctionCall~
    }

    class QuerySpecification {
        +getSelect() Select
        +getHaving() Optional~Expression~
    }

    class Select {
    }

    class Expression {
    }

    class Type {
    }

    class QualifiedName {
    }

    class FunctionCall {
    }

    class Multimap~QualifiedName,Expression~ {
    }

    class Scope {
    }

    StatementAnalyzer --> Analysis : uses
    StatementAnalyzer ..> OrderByExpressionRewriter : creates and uses
    StatementAnalyzer --> ExpressionAnalysis : uses
    StatementAnalyzer --> QuerySpecification : analyzes
    QuerySpecification --> Select : contains
    Analysis --> Expression : stores HAVING
    OrderByExpressionRewriter --> Expression : rewrites
    ExpressionAnalysis --> Type : returns
    ExpressionAnalysis --> FunctionCall : returns
    OrderByExpressionRewriter --> Multimap~QualifiedName,Expression~ : uses
    Multimap~QualifiedName,Expression~ --> QualifiedName : keys
    Multimap~QualifiedName,Expression~ --> Expression : values

Loading

Flow diagram for rewritten HAVING predicate analysis

flowchart TD
    A[Start analyzeHaving for QuerySpecification] --> B{node.hasHaving?}
    B -->|No| Z[Return without HAVING analysis]
    B -->|Yes| C[Read original HAVING predicate]
    C --> D[extractNamedOutputExpressions from SELECT]
    D --> E[Create OrderByExpressionRewriter with clauseName HAVING]
    E --> F[Rewrite HAVING predicate using named output aliases]
    F --> G[Analyze rewrittenPredicate with analyzeExpression]
    G --> H[Record subqueries in Analysis]
    H --> I[Determine type of rewrittenPredicate]
    I --> J{Type is BOOLEAN or UNKNOWN?}
    J -->|No| K[Throw SemanticException for non boolean HAVING]
    J -->|Yes| L[Store rewrittenPredicate in Analysis as HAVING expression]
    L --> M[Return to caller]
    K --> M
    Z --> M

Loading

File-Level Changes

Change	Details	Files
Rewrite HAVING predicates using the same alias-resolution logic as ORDER BY and ensure analysis uses the rewritten expression.	In analyzeHaving, construct namedOutputExpressions from the SELECT list and rewrite the HAVING predicate via OrderByExpressionRewriter before expression analysis. Use the rewritten HAVING predicate for expression analysis, type checking, error reporting, and store it into Analysis via analysis.setHaving. Adjust type retrieval and SemanticException locations to reference the rewritten predicate instead of the original expression.	presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java
Ensure grouping and aggregation analysis sees the rewritten HAVING predicate rather than the original expression when available.	In visitQuerySpecification, when building sourceExpressions, prefer the HAVING expression stored in Analysis if present; fall back to the original HAVING expression otherwise. Pass this updated sourceExpressions list into analyzeGroupingOperations and analyzeAggregations so they consider alias-resolved HAVING expressions.	presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java
Generalize OrderByExpressionRewriter to support different clause names for better error messages and reuse in HAVING.	Extend OrderByExpressionRewriter with a clauseName field and a constructor overload that defaults to "ORDER BY" for existing usages. Update ambiguous attribute error messages to include the generic clause name (e.g., ORDER BY or HAVING) instead of hard-coded ORDER BY. Use the new constructor when rewriting HAVING and pass the clause name string (note: verify spelling of the clause name constant).	presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java
Update analyzer tests to assert successful analysis of HAVING clauses referencing SELECT aliases.	Replace the previous negative test expecting MISSING_ATTRIBUTE for HAVING alias usage with multiple positive analyze() calls that reference SELECT aliases in HAVING. Add coverage for multiple aliases, GROUP BY with aliases, and simple aggregate-alias comparisons in HAVING.	presto-main-base/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 4 issues, and left some high level feedback:

• The OrderByExpressionRewriter usage in analyzeHaving passes the clause name as "HAVNG" instead of "HAVING", which will lead to a misspelled clause name in ambiguity error messages and should be corrected.
• Now that OrderByExpressionRewriter is reused for HAVING predicates, consider renaming it (and its factory usage) to a more generic alias-resolution name to better reflect its broader purpose.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `OrderByExpressionRewriter` usage in `analyzeHaving` passes the clause name as "HAVNG" instead of "HAVING", which will lead to a misspelled clause name in ambiguity error messages and should be corrected.
- Now that `OrderByExpressionRewriter` is reused for HAVING predicates, consider renaming it (and its factory usage) to a more generic alias-resolution name to better reflect its broader purpose.

## Individual Comments

### Comment 1
<location path="presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java" line_range="3823-3825" />
<code_context>
-                ExpressionAnalysis expressionAnalysis = analyzeExpression(predicate, scope);
+                // Reuse OrderByExpressionRewriter to resolve SELECT aliases in HAVING
+                Multimap<QualifiedName, Expression> namedOutputExpressions = extractNamedOutputExpressions(node.getSelect());
+                Expression rewrittenPredicate = ExpressionTreeRewriter.rewriteWith(new OrderByExpressionRewriter(namedOutputExpressions, "HAVNG"), predicate);
+
+                // Analyze the rewritten expression
</code_context>
<issue_to_address>
**issue (typo):** Fix the typo in the clause name string passed to the rewriter ("HAVNG" → "HAVING").

This string is used in the error message `"'%s' in '%s' is ambiguous"`, so keeping it as `"HAVNG"` will produce confusing output like `in 'HAVNG'`. It should be corrected to `"HAVING"` to match the actual clause name.

```suggestion
                // Reuse OrderByExpressionRewriter to resolve SELECT aliases in HAVING
                Multimap<QualifiedName, Expression> namedOutputExpressions = extractNamedOutputExpressions(node.getSelect());
                Expression rewrittenPredicate = ExpressionTreeRewriter.rewriteWith(new OrderByExpressionRewriter(namedOutputExpressions, "HAVING"), predicate);
```
</issue_to_address>

### Comment 2
<location path="presto-main-base/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java" line_range="305-308" />
<code_context>
     {
-        assertFails(MISSING_ATTRIBUTE, "SELECT sum(a) x FROM t1 HAVING x > 5");
+        // HAVING now support referencing SELECT aliases for improved SQL compatibility
+        analyze("SELECT sum(a) x FROM t1 HAVING x > 5");
+        analyze("SELECT sum(a) AS total FROM t1 GROUP BY b HAVING total > 10");
+        analyze("SELECT count(*) AS cnt, sum(a) AS total FROM t1 GROUP BY b HAVING cnt > 5 AND total > 100");
+        analyze("SELECT sum(a) as sum_a FROM t1 GROUP BY b HAVING sum_a > 1");
     }

</code_context>
<issue_to_address>
**suggestion (testing):** Add negative tests for ambiguous alias references in HAVING

Since HAVING now supports SELECT aliases, please also add negative tests for ambiguous aliases, similar to ORDER BY. For instance:

```java
assertFails(AMBIGUOUS_ATTRIBUTE, "SELECT sum(a) x, sum(b) x FROM t1 HAVING x > 5");
```

This ensures multiple definitions of the same alias in the SELECT list cause the expected ambiguity error when referenced from HAVING, and validates the new alias resolution logic and error message for this case.
</issue_to_address>

### Comment 3
<location path="presto-main-base/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java" line_range="306-308" />
<code_context>
-        assertFails(MISSING_ATTRIBUTE, "SELECT sum(a) x FROM t1 HAVING x > 5");
+        // HAVING now support referencing SELECT aliases for improved SQL compatibility
+        analyze("SELECT sum(a) x FROM t1 HAVING x > 5");
+        analyze("SELECT sum(a) AS total FROM t1 GROUP BY b HAVING total > 10");
+        analyze("SELECT count(*) AS cnt, sum(a) AS total FROM t1 GROUP BY b HAVING cnt > 5 AND total > 100");
+        analyze("SELECT sum(a) as sum_a FROM t1 GROUP BY b HAVING sum_a > 1");
     }

</code_context>
<issue_to_address>
**suggestion (testing):** Add a negative test for HAVING referencing a non-existent or non-select alias

With the new positive cases, we should also keep a negative one where HAVING references a non-existent, non-resolvable name (not a SELECT alias), e.g.:

```java
assertFails(MISSING_ATTRIBUTE, "SELECT sum(a) AS total FROM t1 HAVING unknown_alias > 5");
```

This ensures unresolved references in HAVING still produce the expected semantic error.
</issue_to_address>

### Comment 4
<location path="presto-main-base/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java" line_range="307-308" />
<code_context>
+        // HAVING now support referencing SELECT aliases for improved SQL compatibility
+        analyze("SELECT sum(a) x FROM t1 HAVING x > 5");
+        analyze("SELECT sum(a) AS total FROM t1 GROUP BY b HAVING total > 10");
+        analyze("SELECT count(*) AS cnt, sum(a) AS total FROM t1 GROUP BY b HAVING cnt > 5 AND total > 100");
+        analyze("SELECT sum(a) as sum_a FROM t1 GROUP BY b HAVING sum_a > 1");
     }

</code_context>
<issue_to_address>
**suggestion (testing):** Add tests to ensure HAVING cannot indirectly introduce window functions via aliases

Since HAVING now rewrites expressions using SELECT aliases, please add a test that window functions are still rejected when referenced via an alias, e.g.:

```java
assertFails(INVALID_WINDOW_FUNCTION,
        "SELECT row_number() OVER () AS rn FROM t1 GROUP BY b HAVING rn > 1");
```
(or using the same error code used elsewhere in `TestAnalyzer` for window functions in HAVING). This verifies that alias rewriting does not bypass the HAVING window-function restriction.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion (testing): Add negative tests for ambiguous alias references in HAVING

Since HAVING now supports SELECT aliases, please also add negative tests for ambiguous aliases, similar to ORDER BY. For instance:

assertFails(AMBIGUOUS_ATTRIBUTE, "SELECT sum(a) x, sum(b) x FROM t1 HAVING x > 5");

This ensures multiple definitions of the same alias in the SELECT list cause the expected ambiguity error when referenced from HAVING, and validates the new alias resolution logic and error message for this case.

[sourcery-ai]

suggestion (testing): Add a negative test for HAVING referencing a non-existent or non-select alias

With the new positive cases, we should also keep a negative one where HAVING references a non-existent, non-resolvable name (not a SELECT alias), e.g.:

assertFails(MISSING_ATTRIBUTE, "SELECT sum(a) AS total FROM t1 HAVING unknown_alias > 5");

This ensures unresolved references in HAVING still produce the expected semantic error.

[sourcery-ai]

suggestion (testing): Add tests to ensure HAVING cannot indirectly introduce window functions via aliases

Since HAVING now rewrites expressions using SELECT aliases, please add a test that window functions are still rejected when referenced via an alias, e.g.:

assertFails(INVALID_WINDOW_FUNCTION,
        "SELECT row_number() OVER () AS rn FROM t1 GROUP BY b HAVING rn > 1");

(or using the same error code used elsewhere in TestAnalyzer for window functions in HAVING). This verifies that alias rewriting does not bypass the HAVING window-function restriction.

[agrawalreetika]

analyzeHaving() is already called earlier than this. So when would this below else happen? Isn't that unreachable?

[mehradpk]

You're right, that else branch is unreachable since analyzeHaving() always calls analysis.setHaving() when a HAVING clause is present. I've simplified the code to remove the unnecessary null check. Thanks for pointing out!

[agrawalreetika]

lgtm

[hantangwangd]

Thanks @mehradpk, lgtm!

[shelton408]

Will be reverting in #27372 , this change has 2 bugs that are breaking existing queries. Testing on the revert has confirmed that this is the cause of 2 errors, see the PR for more details