Recursively check presence of OVER clause of window functions

trinodb · May 26, 2015 · 50eac1a · 50eac1a
1 parent fbcf3a1
commit 50eac1a
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 9 deletions.
diff --git a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/TupleAnalyzer.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/TupleAnalyzer.java
@@ -116,7 +116,6 @@
 import static com.facebook.presto.sql.analyzer.SemanticErrorCode.VIEW_IS_STALE;
 import static com.facebook.presto.sql.analyzer.SemanticErrorCode.VIEW_PARSE_ERROR;
 import static com.facebook.presto.sql.analyzer.SemanticErrorCode.WILDCARD_WITHOUT_FROM;
-import static com.facebook.presto.sql.analyzer.SemanticErrorCode.WINDOW_REQUIRES_OVER;
 import static com.facebook.presto.sql.planner.ExpressionInterpreter.expressionOptimizer;
 import static com.facebook.presto.sql.tree.ComparisonExpression.Type.EQUAL;
 import static com.facebook.presto.sql.tree.FrameBound.Type.CURRENT_ROW;
@@ -620,14 +619,7 @@ private void analyzeWindowFunctions(QuerySpecification node, List<FieldOrExpress
         for (FieldOrExpression fieldOrExpression : Iterables.concat(outputExpressions, orderByExpressions)) {
             if (fieldOrExpression.isExpression()) {
                 extractor.process(fieldOrExpression.getExpression(), null);
-                if (fieldOrExpression.getExpression() instanceof FunctionCall) {
-                    FunctionCall functionCall = (FunctionCall) fieldOrExpression.getExpression();
-                    FunctionInfo functionInfo = analysis.getFunctionInfo(functionCall);
-                    checkState(functionInfo != null, "functionInfo is null");
-                    if (functionInfo.isWindow() && !functionInfo.isAggregate() && !functionCall.getWindow().isPresent()) {
-                        throw new SemanticException(WINDOW_REQUIRES_OVER, node, "Window function %s requires an OVER clause", functionInfo.getName());
-                    }
-                }
+                new WindowFunctionValidator().process(fieldOrExpression.getExpression(), analysis);
             }
         }
 

diff --git a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/WindowFunctionValidator.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/WindowFunctionValidator.java
@@ -0,0 +1,37 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.facebook.presto.sql.analyzer;
+
+import com.facebook.presto.metadata.FunctionInfo;
+import com.facebook.presto.sql.tree.DefaultExpressionTraversalVisitor;
+import com.facebook.presto.sql.tree.FunctionCall;
+
+import static com.facebook.presto.sql.analyzer.SemanticErrorCode.WINDOW_REQUIRES_OVER;
+import static com.google.common.base.Preconditions.checkNotNull;
+
+public class WindowFunctionValidator
+        extends DefaultExpressionTraversalVisitor<Void, Analysis>
+{
+    @Override
+    protected Void visitFunctionCall(FunctionCall functionCall, Analysis analysis)
+    {
+        checkNotNull(analysis, "analysis is null");
+
+        FunctionInfo functionInfo = analysis.getFunctionInfo(functionCall);
+        if (functionInfo != null && functionInfo.isWindow() && !functionInfo.isAggregate() && !functionCall.getWindow().isPresent()) {
+            throw new SemanticException(WINDOW_REQUIRES_OVER, functionCall, "Window function %s requires an OVER clause", functionInfo.getName());
+        }
+        return super.visitFunctionCall(functionCall, analysis);
+    }
+}
diff --git a/presto-main/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java b/presto-main/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java
@@ -407,6 +407,7 @@ public void testNestedWindowFunctions()
     public void testWindowFunctionWithoutOverClause()
     {
         assertFails(WINDOW_REQUIRES_OVER, "SELECT row_number()");
+        assertFails(WINDOW_REQUIRES_OVER, "SELECT coalesce(lead(a), 0) from (values(0)) t(a)");
     }
 
     @Test