Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hantangwangd merged 1 commit into from
Mar 11, 2026
+47 −7
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2967,7 +2967,10 @@ protected Scope visitQuerySpecification(QuerySpecification node, Optional<Scope>
analysis.setOrderByExpressions(node, orderByExpressions);

List<Expression> sourceExpressions = new ArrayList<>(outputExpressions);
node.getHaving().ifPresent(sourceExpressions::add);
// Use the rewritten HAVING expression (to resolve SELECT alias references)
if (node.getHaving().isPresent()) {
sourceExpressions.add(analysis.getHaving(node));
}

analyzeGroupingOperations(node, sourceExpressions, orderByExpressions);
List<FunctionCall> aggregates = analyzeAggregations(node, sourceExpressions, orderByExpressions);
Expand Down Expand Up @@ -3820,7 +3823,12 @@ private void analyzeHaving(QuerySpecification node, Scope scope)
if (node.getHaving().isPresent()) {
Expression predicate = node.getHaving().get();

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, "HAVING"), predicate);

// Analyze the rewritten expression
ExpressionAnalysis expressionAnalysis = analyzeExpression(rewrittenPredicate, scope);

expressionAnalysis.getWindowFunctions().stream()
.findFirst()
Expand All @@ -3830,12 +3838,12 @@ private void analyzeHaving(QuerySpecification node, Scope scope)

analysis.recordSubqueries(node, expressionAnalysis);

Type predicateType = expressionAnalysis.getType(predicate);
Type predicateType = expressionAnalysis.getType(rewrittenPredicate);
if (!predicateType.equals(BOOLEAN) && !predicateType.equals(UNKNOWN)) {
throw new SemanticException(TYPE_MISMATCH, predicate, "HAVING clause must evaluate to a boolean: actual type %s", predicateType);
throw new SemanticException(TYPE_MISMATCH, rewrittenPredicate, "HAVING clause must evaluate to a boolean: actual type %s", predicateType);
}

analysis.setHaving(node, predicate);
analysis.setHaving(node, rewrittenPredicate);
}
}

Expand Down Expand Up @@ -3886,10 +3894,17 @@ private class OrderByExpressionRewriter
extends ExpressionRewriter<Void>
{
private final Multimap<QualifiedName, Expression> assignments;
private final String clauseName;

public OrderByExpressionRewriter(Multimap<QualifiedName, Expression> assignments)
{
this(assignments, "ORDER BY");
}

public OrderByExpressionRewriter(Multimap<QualifiedName, Expression> assignments, String clauseName)
{
this.assignments = assignments;
this.clauseName = clauseName;
}

@Override
Expand All @@ -3902,7 +3917,7 @@ public Expression rewriteIdentifier(Identifier reference, Void context, Expressi
.collect(Collectors.toSet());

if (expressions.size() > 1) {
throw new SemanticException(AMBIGUOUS_ATTRIBUTE, reference, "'%s' in ORDER BY is ambiguous", name);
throw new SemanticException(AMBIGUOUS_ATTRIBUTE, reference, "'%s' in '%s' is ambiguous", name, clauseName);
}

if (expressions.size() == 1) {
Expand Down
27 changes: 26 additions & 1 deletion presto-main-base/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,32 @@ public void testReferenceToOutputColumnFromOrderByAggregation()
@Test
public void testHavingReferencesOutputAlias()
{
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");
Comment on lines +312 to +315
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +313 to +315
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +314 to +315
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

@Test
public void testHavingAmbiguousAlias()
{
// Ambiguous alias referenced in HAVING should throw appropriate error
assertFails(AMBIGUOUS_ATTRIBUTE, "SELECT sum(a) AS x, count(b) AS x FROM t1 GROUP BY c HAVING x > 5");
}

@Test
public void testHavingNonExistentAlias()
{
// Non-existent alias in HAVING should fail with MISSING_ATTRIBUTE
assertFails(MISSING_ATTRIBUTE, "SELECT sum(a) AS total FROM t1 GROUP BY b HAVING unknown_alias > 5");
}

@Test
public void testHavingWindowFunctionViaAlias()
{
// Window functions are not allowed in HAVING, even when referenced via alias
assertFails(NESTED_WINDOW, "SELECT row_number() OVER () AS rn FROM t1 GROUP BY b HAVING rn > 1");
}

@Test
Expand Down
Loading