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 4fe1e2f3e5f4e..09fe0e2e39b7e 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,10 +2967,7 @@ protected Scope visitQuerySpecification(QuerySpecification node, Optional analysis.setOrderByExpressions(node, orderByExpressions); List sourceExpressions = new ArrayList<>(outputExpressions); - // Use the rewritten HAVING expression (to resolve SELECT alias references) - if (node.getHaving().isPresent()) { - sourceExpressions.add(analysis.getHaving(node)); - } + node.getHaving().ifPresent(sourceExpressions::add); analyzeGroupingOperations(node, sourceExpressions, orderByExpressions); List aggregates = analyzeAggregations(node, sourceExpressions, orderByExpressions); @@ -3823,12 +3820,7 @@ private void analyzeHaving(QuerySpecification node, Scope scope) if (node.getHaving().isPresent()) { Expression predicate = node.getHaving().get(); - // 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 expressionAnalysis = analyzeExpression(predicate, scope); expressionAnalysis.getWindowFunctions().stream() .findFirst() @@ -3838,12 +3830,12 @@ private void analyzeHaving(QuerySpecification node, Scope scope) analysis.recordSubqueries(node, expressionAnalysis); - Type predicateType = expressionAnalysis.getType(rewrittenPredicate); + Type predicateType = expressionAnalysis.getType(predicate); if (!predicateType.equals(BOOLEAN) && !predicateType.equals(UNKNOWN)) { - throw new SemanticException(TYPE_MISMATCH, rewrittenPredicate, "HAVING clause must evaluate to a boolean: actual type %s", predicateType); + throw new SemanticException(TYPE_MISMATCH, predicate, "HAVING clause must evaluate to a boolean: actual type %s", predicateType); } - analysis.setHaving(node, rewrittenPredicate); + analysis.setHaving(node, predicate); } } @@ -3894,17 +3886,10 @@ 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 @@ -3917,7 +3902,7 @@ public Expression rewriteIdentifier(Identifier reference, Void context, Expressi .collect(Collectors.toSet()); if (expressions.size() > 1) { - throw new SemanticException(AMBIGUOUS_ATTRIBUTE, reference, "'%s' in '%s' is ambiguous", name, clauseName); + throw new SemanticException(AMBIGUOUS_ATTRIBUTE, reference, "'%s' in ORDER BY is ambiguous", name); } 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 f6bfcc5d75e58..eeef49e6805f5 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,32 +308,7 @@ public void testReferenceToOutputColumnFromOrderByAggregation() @Test public void testHavingReferencesOutputAlias() { - // 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"); + assertFails(MISSING_ATTRIBUTE, "SELECT sum(a) x FROM t1 HAVING x > 5"); } @Test