diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java b/presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java index 09fe0e2e39b7e..4fe1e2f3e5f4e 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java @@ -2967,7 +2967,10 @@ protected Scope visitQuerySpecification(QuerySpecification node, Optional analysis.setOrderByExpressions(node, orderByExpressions); List 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 aggregates = analyzeAggregations(node, sourceExpressions, orderByExpressions); @@ -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 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() @@ -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); } } @@ -3886,10 +3894,17 @@ private class OrderByExpressionRewriter extends ExpressionRewriter { private final Multimap assignments; + private final String clauseName; public OrderByExpressionRewriter(Multimap assignments) + { + this(assignments, "ORDER BY"); + } + + public OrderByExpressionRewriter(Multimap assignments, String clauseName) { this.assignments = assignments; + this.clauseName = clauseName; } @Override @@ -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) { diff --git a/presto-main-base/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java b/presto-main-base/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java index eeef49e6805f5..f6bfcc5d75e58 100644 --- a/presto-main-base/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java +++ b/presto-main-base/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java @@ -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"); + } + + @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