You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by im...@apache.org on 2021/09/23 03:01:08 UTC

[asterixdb] branch master updated: [ASTERIXDB-2968] Fix mixing SQL++ and External UDFs

This is an automated email from the ASF dual-hosted git repository.

imaxon 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 04c4e23  [ASTERIXDB-2968] Fix mixing SQL++ and External UDFs
04c4e23 is described below

commit 04c4e23df4c1d005f62cfbfbb5b13ee89d0d5924
Author: Ian Maxon <ia...@maxons.email>
AuthorDate: Mon Sep 20 12:13:14 2021 -0700

    [ASTERIXDB-2968] Fix mixing SQL++ and External UDFs
    
    - user model changes: no
    - storage format changes: no
    - interface changes: no
    
    Details:
    
    In the inlining of views and SQL++ UDFs, one of the cases
    assumes that if a function is not builtin, it must be a SQL++
    UDF. And therefore if it is not in the used list of those, it
    is missing somehow. This however is the exact case for external
    functions, which shouldn't be inlined (at least in the same way)
    
    Change-Id: Idb8d6b5f7876f42169b1c2b92acc586bfaede5da
    Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13284
    Reviewed-by: Ian Maxon <im...@uci.edu>
    Reviewed-by: Dmitry Lychagin <dm...@couchbase.com>
    Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Contrib: Ian Maxon <im...@uci.edu>
---
 .../mixedudfs/mixedudfs.0.ddl.sqlpp                | 26 ++++++++++++++++++++++
 .../mixedudfs/mixedudfs.1.lib.sqlpp                | 19 ++++++++++++++++
 .../mixedudfs/mixedudfs.2.ddl.sqlpp                | 25 +++++++++++++++++++++
 .../mixedudfs/mixedudfs.3.query.sqlpp              | 23 +++++++++++++++++++
 .../mixedudfs/mixedudfs.4.ddl.sqlpp                | 19 ++++++++++++++++
 .../external-library/mixedudfs/getCapital.1.adm    |  6 +++++
 .../resources/runtimets/testsuite_it_sqlpp.xml     |  5 +++++
 .../common/visitor/AbstractInlineUdfsVisitor.java  |  3 ++-
 8 files changed, 125 insertions(+), 1 deletion(-)

diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.0.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.0.ddl.sqlpp
new file mode 100644
index 0000000..5a5bbec
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.0.ddl.sqlpp
@@ -0,0 +1,26 @@
+/*
+ * 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.
+ */
+DROP DATAVERSE externallibtest if exists;
+CREATE DATAVERSE  externallibtest;
+USE externallibtest;
+
+create type CountryCapitalType if not exists as closed {
+country: string,
+capital: string
+};
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.1.lib.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.1.lib.sqlpp
new file mode 100644
index 0000000..cc6c855
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.1.lib.sqlpp
@@ -0,0 +1,19 @@
+/*
+ * 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.
+ */
+install externallibtest testlib java admin admin target/data/externallib/asterix-external-data-testlib.zip
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.2.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.2.ddl.sqlpp
new file mode 100644
index 0000000..4ef5d6b
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.2.ddl.sqlpp
@@ -0,0 +1,25 @@
+/*
+ * 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.
+ */
+
+use externallibtest;
+
+create function getCapital(a: string) returns CountryCapitalType
+  as "org.apache.asterix.external.library.CapitalFinderFactory" at testlib;
+
+create function mysum(a, b) { a+b };
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.3.query.sqlpp
new file mode 100644
index 0000000..cbf2a35
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.3.query.sqlpp
@@ -0,0 +1,23 @@
+/*
+ * 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.
+ */
+use externallibtest;
+
+SELECT getCapital(country) as natCap, mysum(1,2) as sum
+FROM ["England","Italy","China","United States","India","Jupiter"] as country
+ORDER BY natCap.country;
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.4.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.4.ddl.sqlpp
new file mode 100644
index 0000000..65733e4
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mixedudfs/mixedudfs.4.ddl.sqlpp
@@ -0,0 +1,19 @@
+/*
+ * 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.
+ */
+DROP DATAVERSE externallibtest;
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/mixedudfs/getCapital.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/mixedudfs/getCapital.1.adm
new file mode 100644
index 0000000..dd2dfbc
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/mixedudfs/getCapital.1.adm
@@ -0,0 +1,6 @@
+{ "natCap": { "country": "China", "capital": "Beijing" }, "sum": 3 }
+{ "natCap": { "country": "England", "capital": "London" }, "sum": 3 }
+{ "natCap": { "country": "India", "capital": "New Delhi" }, "sum": 3 }
+{ "natCap": { "country": "Italy", "capital": "Rome" }, "sum": 3 }
+{ "natCap": { "country": "Jupiter", "capital": "NOT_FOUND" }, "sum": 3 }
+{ "natCap": { "country": "United States", "capital": "Washington D.C." }, "sum": 3 }
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_it_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_it_sqlpp.xml
index 85fe2c5..7667bb4 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_it_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_it_sqlpp.xml
@@ -108,6 +108,11 @@
       </compilation-unit>
     </test-case>
     <test-case FilePath="external-library">
+      <compilation-unit name="mixedudfs">
+        <output-dir compare="Text">mixedudfs</output-dir>
+      </compilation-unit>
+    </test-case>
+    <test-case FilePath="external-library">
       <compilation-unit name="udf_metadata">
         <output-dir compare="Text">udf_metadata</output-dir>
       </compilation-unit>
diff --git a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/AbstractInlineUdfsVisitor.java b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/AbstractInlineUdfsVisitor.java
index 7fe7140..52775d3 100644
--- a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/AbstractInlineUdfsVisitor.java
+++ b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/AbstractInlineUdfsVisitor.java
@@ -316,7 +316,8 @@ public abstract class AbstractInlineUdfsVisitor extends AbstractQueryExpressionV
         } else {
             FunctionDecl implem = usedUDFs.get(fs);
             if (implem == null) {
-                throw new CompilationException(ErrorCode.UNKNOWN_FUNCTION, f.getSourceLocation(), fs.toString());
+                //it's an external UDF
+                return new Pair<>(r, expr);
             }
             // it's one of the functions we want to inline
             boolean isVarargs = implem.getSignature().getArity() == FunctionIdentifier.VARARGS;