[ASTERIXDB-2777][COMP] Fix CREATE FUNCTION failure
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
- Fixed illegal state error with CREATE FUNCTION when
a function parameter is used in HAVING clause
Change-Id: Ic1ceab1ff39d7d4512c83fabab894ca301afd79a
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/7863
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Dmitry Lychagin <dmitry.lychagin@couchbase.com>
Reviewed-by: Ali Alsuliman <ali.al.solaiman@gmail.com>
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/user-defined-functions/udf34/udf34.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/user-defined-functions/udf34/udf34.1.ddl.sqlpp
new file mode 100644
index 0000000..6b0e577
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/user-defined-functions/udf34/udf34.1.ddl.sqlpp
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+/*
+ * Description : Function parameter used in HAVING clause
+ * Expected Res : Success
+ */
+
+drop dataverse test if exists;
+create dataverse test;
+
+create function test.f1(v) {
+ select value x
+ from range(1, 9) r
+ group by r % 3 as x
+ having count(*) > v
+ order by x
+};
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/user-defined-functions/udf34/udf34.2.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/user-defined-functions/udf34/udf34.2.query.sqlpp
new file mode 100644
index 0000000..8fcb9a2
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/user-defined-functions/udf34/udf34.2.query.sqlpp
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+/*
+ * Description : Function parameter used in HAVING clause
+ * Expected Res : Success
+ */
+
+test.f1(2);
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/user-defined-functions/udf34/udf34.2.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/user-defined-functions/udf34/udf34.2.adm
new file mode 100644
index 0000000..c55eb7a
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/user-defined-functions/udf34/udf34.2.adm
@@ -0,0 +1,3 @@
+0
+1
+2
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
index 91abbea..afdedff 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -12046,6 +12046,11 @@
</compilation-unit>
</test-case>
<test-case FilePath="user-defined-functions">
+ <compilation-unit name="udf34">
+ <output-dir compare="Text">udf34</output-dir>
+ </compilation-unit>
+ </test-case>
+ <test-case FilePath="user-defined-functions">
<compilation-unit name="f01">
<output-dir compare="Text">f01</output-dir>
<expected-error>ASX1081: Cannot find function with name test.tinyint</expected-error>
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java
index 3c98a2d..3f93dd0 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java
@@ -20,6 +20,7 @@
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@@ -113,7 +114,7 @@
this.context = context;
this.declaredFunctions = declaredFunctions;
this.metadataProvider = metadataProvider;
- this.externalVars = externalVars;
+ this.externalVars = externalVars != null ? externalVars : Collections.emptyList();
this.isLogEnabled = LOGGER.isTraceEnabled();
logExpression("Starting AST rewrites on", "");
}
@@ -197,7 +198,7 @@
}
protected void rewriteGroupByAggregationSugar() throws CompilationException {
- SqlppGroupByAggregationSugarVisitor visitor = new SqlppGroupByAggregationSugarVisitor(context);
+ SqlppGroupByAggregationSugarVisitor visitor = new SqlppGroupByAggregationSugarVisitor(context, externalVars);
rewriteTopExpr(visitor, null);
}
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupByAggregationSugarVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupByAggregationSugarVisitor.java
index baf3f46..f4c8085 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupByAggregationSugarVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupByAggregationSugarVisitor.java
@@ -40,6 +40,7 @@
import org.apache.asterix.lang.common.expression.VariableExpr;
import org.apache.asterix.lang.common.rewrites.LangRewritingContext;
import org.apache.asterix.lang.common.struct.Identifier;
+import org.apache.asterix.lang.common.struct.VarIdentifier;
import org.apache.asterix.lang.sqlpp.clause.FromClause;
import org.apache.asterix.lang.sqlpp.clause.SelectBlock;
import org.apache.asterix.lang.sqlpp.clause.SelectClause;
@@ -81,8 +82,11 @@
*/
public class SqlppGroupByAggregationSugarVisitor extends AbstractSqlppExpressionScopingVisitor {
- public SqlppGroupByAggregationSugarVisitor(LangRewritingContext context) {
+ private final Collection<VarIdentifier> externalVars;
+
+ public SqlppGroupByAggregationSugarVisitor(LangRewritingContext context, Collection<VarIdentifier> externalVars) {
super(context);
+ this.externalVars = externalVars;
}
@Override
@@ -177,7 +181,7 @@
// Gets the final free variables.
freeVariables.addAll(freeVariablesInGbyLets);
- freeVariables.removeIf(SqlppVariableUtil::isExternalVariableReference);
+ removeExternalVariables(freeVariables);
if (!groupbyClause.isGroupAll()) {
// Gets outer scope variables.
@@ -213,6 +217,12 @@
return null;
}
+ private void removeExternalVariables(Collection<VariableExpr> freeVariables) {
+ if (!externalVars.isEmpty()) {
+ freeVariables.removeIf(ve -> externalVars.contains(ve.getVar()));
+ }
+ }
+
static Map<VariableExpr, Identifier> createGroupVarFieldMap(List<Pair<Expression, Identifier>> fieldList) {
Map<VariableExpr, Identifier> fieldVars = new HashMap<>();
for (Pair<Expression, Identifier> p : fieldList) {