This is an automated email from the ASF dual-hosted git repository.
dlych pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git
The following commit(s) were added to refs/heads/master by this push:
new 7572916 [ASTERIXDB-2777][COMP] Fix CREATE FUNCTION failure
7572916 is described below
commit 75729167ec0b034c9b08a781eba6b55b8ea18f21
Author: Dmitry Lychagin <[email protected]>
AuthorDate: Tue Sep 8 13:36:12 2020 -0700
[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 <[email protected]>
Tested-by: Jenkins <[email protected]>
Reviewed-by: Dmitry Lychagin <[email protected]>
Reviewed-by: Ali Alsuliman <[email protected]>
---
.../user-defined-functions/udf34/udf34.1.ddl.sqlpp | 33 ++++++++++++++++++++++
.../udf34/udf34.2.query.sqlpp | 24 ++++++++++++++++
.../user-defined-functions/udf34/udf34.2.adm | 3 ++
.../test/resources/runtimets/testsuite_sqlpp.xml | 5 ++++
.../lang/sqlpp/rewrites/SqlppQueryRewriter.java | 5 ++--
.../SqlppGroupByAggregationSugarVisitor.java | 14 +++++++--
6 files changed, 80 insertions(+), 4 deletions(-)
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 @@ package org.apache.asterix.lang.sqlpp.rewrites;
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 @@ public class SqlppQueryRewriter implements IQueryRewriter {
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 @@ public class SqlppQueryRewriter implements IQueryRewriter {
}
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.GbyVariableExpressionPair;
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 @@ import org.apache.hyracks.algebricks.common.utils.Pair;
*/
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 @@ public class SqlppGroupByAggregationSugarVisitor extends
AbstractSqlppExpression
// 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 @@ public class SqlppGroupByAggregationSugarVisitor extends
AbstractSqlppExpression
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) {
