You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2018/08/13 01:06:03 UTC

[3/4] calcite git commit: [CALCITE-2446] Lateral joins do not work when saved as custom views (Piotr Bojko)

[CALCITE-2446] Lateral joins do not work when saved as custom views (Piotr Bojko)

When expanding views, Calcite had one additional step in which
expanded sqlnode tree was validated and thus rewritten. During this
step - information about lateral join was lost.

This change removes validation step from view expansion algorithms.

Also, added bug reproduction in form of unit tests.

Close apache/calcite#780


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/55748738
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/55748738
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/55748738

Branch: refs/heads/master
Commit: 5574873844a440d2ed14dfdc7b0e40a0dae91355
Parents: ac33200
Author: Piotr Bojko <pt...@gmail.com>
Authored: Fri Aug 3 16:35:19 2018 +0200
Committer: Julian Hyde <jh...@apache.org>
Committed: Sun Aug 12 15:16:51 2018 -0700

----------------------------------------------------------------------
 .../calcite/prepare/CalcitePrepareImpl.java     |  3 +-
 .../org/apache/calcite/prepare/PlannerImpl.java |  3 +-
 .../org/apache/calcite/test/CalciteAssert.java  | 34 +++++++++-
 .../java/org/apache/calcite/test/JdbcTest.java  | 30 +++++++++
 .../java/org/apache/calcite/util/Smalls.java    |  7 +++
 .../apache/calcite/chinook/CodesFunction.java   | 66 ++++++++++++++++++++
 plus/src/main/resources/chinook/chinook.json    | 45 ++++++++++---
 .../test/resources/sql/cross-join-lateral.iq    | 55 ++++++++++++++++
 8 files changed, 231 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/55748738/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java b/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
index 5a6d521..b6023d0 100644
--- a/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
+++ b/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
@@ -1179,13 +1179,12 @@ public class CalcitePrepareImpl implements CalcitePrepare {
       final CatalogReader catalogReader =
           this.catalogReader.withSchemaPath(schemaPath);
       SqlValidator validator = createSqlValidator(catalogReader);
-      SqlNode sqlNode1 = validator.validate(sqlNode);
       final SqlToRelConverter.Config config = SqlToRelConverter.configBuilder()
               .withTrimUnusedFields(true).build();
       SqlToRelConverter sqlToRelConverter =
           getSqlToRelConverter(validator, catalogReader, config);
       RelRoot root =
-          sqlToRelConverter.convertQuery(sqlNode1, true, false);
+          sqlToRelConverter.convertQuery(sqlNode, true, false);
 
       --expansionDepth;
       return root;

http://git-wip-us.apache.org/repos/asf/calcite/blob/55748738/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java b/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java
index cb27ea2..6f746ab 100644
--- a/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java
+++ b/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java
@@ -262,7 +262,6 @@ public class PlannerImpl implements Planner {
           new CalciteSqlValidator(operatorTable, catalogReader, typeFactory,
               conformance);
       validator.setIdentifierExpansion(true);
-      final SqlNode validatedSqlNode = validator.validate(sqlNode);
 
       final RexBuilder rexBuilder = createRexBuilder();
       final RelOptCluster cluster = RelOptCluster.create(planner, rexBuilder);
@@ -276,7 +275,7 @@ public class PlannerImpl implements Planner {
           new SqlToRelConverter(new ViewExpanderImpl(), validator,
               catalogReader, cluster, convertletTable, config);
 
-      root = sqlToRelConverter.convertQuery(validatedSqlNode, true, false);
+      root = sqlToRelConverter.convertQuery(sqlNode, true, false);
       root = root.withRel(sqlToRelConverter.flattenTypes(root.rel, true));
       final RelBuilder relBuilder =
           config.getRelBuilderFactory().create(cluster, null);

http://git-wip-us.apache.org/repos/asf/calcite/blob/55748738/core/src/test/java/org/apache/calcite/test/CalciteAssert.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/CalciteAssert.java b/core/src/test/java/org/apache/calcite/test/CalciteAssert.java
index 171fed4..3d7ce4e 100644
--- a/core/src/test/java/org/apache/calcite/test/CalciteAssert.java
+++ b/core/src/test/java/org/apache/calcite/test/CalciteAssert.java
@@ -38,7 +38,9 @@ import org.apache.calcite.runtime.GeoFunctions;
 import org.apache.calcite.runtime.Hook;
 import org.apache.calcite.schema.Schema;
 import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.TableFunction;
 import org.apache.calcite.schema.impl.AbstractSchema;
+import org.apache.calcite.schema.impl.TableFunctionImpl;
 import org.apache.calcite.schema.impl.ViewTable;
 import org.apache.calcite.schema.impl.ViewTableMacro;
 import org.apache.calcite.sql.validate.SqlConformanceEnum;
@@ -49,6 +51,7 @@ import org.apache.calcite.util.Closer;
 import org.apache.calcite.util.Holder;
 import org.apache.calcite.util.JsonBuilder;
 import org.apache.calcite.util.Pair;
+import org.apache.calcite.util.Smalls;
 import org.apache.calcite.util.Util;
 
 import org.apache.commons.dbcp2.PoolableConnectionFactory;
@@ -775,6 +778,28 @@ public class CalciteAssert {
               ImmutableList.of(), ImmutableList.of("POST", "EMPS"),
               null));
       return post;
+    case AUX:
+      SchemaPlus aux =
+          rootSchema.add(schema.schemaName, new AbstractSchema());
+      TableFunction tableFunction =
+          TableFunctionImpl.create(Smalls.SimpleTableFunction.class, "eval");
+      aux.add("TBLFUN", tableFunction);
+      final String simpleSql = "select *\n"
+          + "from (values\n"
+          + "    ('ABC', 1),\n"
+          + "    ('DEF', 2),\n"
+          + "    ('GHI', 3))\n"
+          + "  as t(strcol, intcol)";
+      aux.add("SIMPLETABLE",
+          ViewTable.viewMacro(aux, simpleSql, ImmutableList.of(),
+              ImmutableList.of("AUX", "SIMPLETABLE"), null));
+      final String lateralSql = "SELECT *\n"
+          + "FROM AUX.SIMPLETABLE ST\n"
+          + "CROSS JOIN LATERAL TABLE(AUX.TBLFUN(ST.INTCOL))";
+      aux.add("VIEWLATERAL",
+          ViewTable.viewMacro(aux, lateralSql, ImmutableList.of(),
+              ImmutableList.of("AUX", "VIEWLATERAL"), null));
+      return aux;
     default:
       throw new AssertionError("unknown schema " + schema);
     }
@@ -860,6 +885,8 @@ public class CalciteAssert {
         return with(SchemaSpec.SCOTT);
       case SPARK:
         return with(CalciteConnectionProperty.SPARK, true);
+      case AUX:
+        return with(SchemaSpec.AUX, SchemaSpec.POST);
       default:
         throw Util.unexpected(config);
       }
@@ -1660,6 +1687,10 @@ public class CalciteAssert {
 
     /** Configuration that loads Spark. */
     SPARK,
+
+    /** Configuration that loads AUX schema for tests involving view expansions
+     * and lateral joins tests. */
+    AUX
   }
 
   /** Implementation of {@link AssertQuery} that does nothing. */
@@ -1779,7 +1810,8 @@ public class CalciteAssert {
     BLANK("BLANK"),
     LINGUAL("SALES"),
     POST("POST"),
-    ORINOCO("ORINOCO");
+    ORINOCO("ORINOCO"),
+    AUX("AUX");
 
     /** The name of the schema that is usually created from this specification.
      * (Names are not unique, and you can use another name if you wish.) */

http://git-wip-us.apache.org/repos/asf/calcite/blob/55748738/core/src/test/java/org/apache/calcite/test/JdbcTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
index 94507b5..70ffe1d 100644
--- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
+++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
@@ -6230,6 +6230,36 @@ public class JdbcTest {
         .throws_("No match found for function signature NVL(<NUMERIC>, <NUMERIC>)");
   }
 
+  /** Unit test for LATERAL CROSS JOIN to table function. */
+  @Test public void testLateralJoin() {
+    final String sql = "SELECT *\n"
+        + "FROM AUX.SIMPLETABLE ST\n"
+        + "CROSS JOIN LATERAL TABLE(AUX.TBLFUN(ST.INTCOL))";
+    CalciteAssert.that(CalciteAssert.Config.AUX)
+        .query(sql)
+        .returnsUnordered(
+            "STRCOL=ABC; INTCOL=1; n=0; s=",
+            "STRCOL=DEF; INTCOL=2; n=0; s=",
+            "STRCOL=DEF; INTCOL=2; n=1; s=a",
+            "STRCOL=GHI; INTCOL=3; n=0; s=",
+            "STRCOL=GHI; INTCOL=3; n=1; s=a",
+            "STRCOL=GHI; INTCOL=3; n=2; s=ab");
+  }
+
+  /** Unit test for view expansion with lateral join. */
+  @Test public void testExpandViewWithLateralJoin() {
+    final String sql = "SELECT * FROM AUX.VIEWLATERAL";
+    CalciteAssert.that(CalciteAssert.Config.AUX)
+        .query(sql)
+        .returnsUnordered(
+            "STRCOL=ABC; INTCOL=1; n=0; s=",
+            "STRCOL=DEF; INTCOL=2; n=0; s=",
+            "STRCOL=DEF; INTCOL=2; n=1; s=a",
+            "STRCOL=GHI; INTCOL=3; n=0; s=",
+            "STRCOL=GHI; INTCOL=3; n=1; s=a",
+            "STRCOL=GHI; INTCOL=3; n=2; s=ab");
+  }
+
   /** Tests that {@link Hook#PARSE_TREE} works. */
   @Test public void testHook() {
     final int[] callCount = {0};

http://git-wip-us.apache.org/repos/asf/calcite/blob/55748738/core/src/test/java/org/apache/calcite/util/Smalls.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/util/Smalls.java b/core/src/test/java/org/apache/calcite/util/Smalls.java
index 46ae6a3..4825f75 100644
--- a/core/src/test/java/org/apache/calcite/util/Smalls.java
+++ b/core/src/test/java/org/apache/calcite/util/Smalls.java
@@ -812,6 +812,13 @@ public class Smalls {
   }
 
   /** A table function that returns a {@link QueryableTable}. */
+  public static class SimpleTableFunction {
+    public QueryableTable eval(Integer s) {
+      return generateStrings(s);
+    }
+  }
+
+  /** A table function that returns a {@link QueryableTable}. */
   public static class MyTableFunction {
     public QueryableTable eval(String s) {
       return oneThreePlus(s);

http://git-wip-us.apache.org/repos/asf/calcite/blob/55748738/plus/src/main/java/org/apache/calcite/chinook/CodesFunction.java
----------------------------------------------------------------------
diff --git a/plus/src/main/java/org/apache/calcite/chinook/CodesFunction.java b/plus/src/main/java/org/apache/calcite/chinook/CodesFunction.java
new file mode 100644
index 0000000..721bf42
--- /dev/null
+++ b/plus/src/main/java/org/apache/calcite/chinook/CodesFunction.java
@@ -0,0 +1,66 @@
+/*
+ * 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.
+ */
+package org.apache.calcite.chinook;
+
+import org.apache.calcite.adapter.java.AbstractQueryableTable;
+import org.apache.calcite.linq4j.Linq4j;
+import org.apache.calcite.linq4j.QueryProvider;
+import org.apache.calcite.linq4j.Queryable;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.schema.QueryableTable;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.sql.type.SqlTypeName;
+
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
+
+/**
+ * Example Table Function for lateral join checks
+ */
+public class CodesFunction {
+
+  private CodesFunction(){
+  }
+
+  public static QueryableTable getTable(String name) {
+
+    return new AbstractQueryableTable(Object[].class) {
+      @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+        return typeFactory.builder()
+            .add("TYPE", SqlTypeName.VARCHAR)
+            .add("CODEVALUE", SqlTypeName.VARCHAR)
+            .build();
+      }
+
+      @Override public Queryable<String[]> asQueryable(QueryProvider queryProvider,
+                                                       SchemaPlus schema,
+                                                       String tableName) {
+        if (name == null) {
+          return Linq4j.<String[]>emptyEnumerable().asQueryable();
+        }
+        return Linq4j.asEnumerable(new String[][]{
+            new String[]{"HASHCODE", "" + name.hashCode()},
+            new String[]{"BASE64",
+                Base64.getEncoder().encodeToString(name.getBytes(StandardCharsets.UTF_8))}
+        }).asQueryable();
+      }
+    };
+  }
+}
+
+// End CodesFunction.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/55748738/plus/src/main/resources/chinook/chinook.json
----------------------------------------------------------------------
diff --git a/plus/src/main/resources/chinook/chinook.json b/plus/src/main/resources/chinook/chinook.json
index e8fee72..afec908 100644
--- a/plus/src/main/resources/chinook/chinook.json
+++ b/plus/src/main/resources/chinook/chinook.json
@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
- {
+{
   "version": "1.0",
   "defaultSchema": "ENHANCED",
   "schemas": [
@@ -33,9 +33,9 @@
       "operand": {},
       "tables": [
         {
-          "name" : "PREFERRED_TRACKS",
-          "type" : "view",
-          "sql" : [
+          "name": "PREFERRED_TRACKS",
+          "type": "view",
+          "sql": [
             "SELECT trackid, name, albumid, mediatypeid, genreid, composer, milliseconds, bytes, unitprice ",
             "FROM chinook.track AS tr",
             "WHERE tr.genreid IN (SELECT id FROM preferred_genres) ",
@@ -43,9 +43,9 @@
           ]
         },
         {
-          "name" : "SIMPLE_CUSTOMER",
-          "type" : "view",
-          "sql" : [
+          "name": "SIMPLE_CUSTOMER",
+          "type": "view",
+          "sql": [
             "SELECT c.firstname, c.lastname, c.email ",
             "FROM chinook.customer AS c"
           ]
@@ -71,6 +71,37 @@
           "className": "org.apache.calcite.chinook.ChosenCustomerEmail"
         }
       ]
+    },
+    {
+      "name": "AUX",
+      "type": "custom",
+      "factory": "org.apache.calcite.schema.impl.AbstractSchema$Factory",
+      "operand": {},
+      "functions": [
+        {
+          "name": "CODES",
+          "className": "org.apache.calcite.chinook.CodesFunction",
+          "methodName": "getTable"
+        }
+      ]
+    },
+    {
+      "name": "EXAMPLES",
+      "type": "custom",
+      "factory": "org.apache.calcite.schema.impl.AbstractSchema$Factory",
+      "operand": {},
+      "tables": [
+        {
+          "name": "CODED_EMAILS",
+          "type": "view",
+          "sql": [
+            "SELECT SC.email, TF.TYPE, TF.CODEVALUE ",
+            "FROM ENHANCED.SIMPLE_CUSTOMER SC ",
+            "CROSS JOIN LATERAL TABLE(AUX.CODES(SC.email)) TF ",
+            "limit 6"
+          ]
+        }
+      ]
     }
   ]
 }

http://git-wip-us.apache.org/repos/asf/calcite/blob/55748738/plus/src/test/resources/sql/cross-join-lateral.iq
----------------------------------------------------------------------
diff --git a/plus/src/test/resources/sql/cross-join-lateral.iq b/plus/src/test/resources/sql/cross-join-lateral.iq
new file mode 100644
index 0000000..98928e5
--- /dev/null
+++ b/plus/src/test/resources/sql/cross-join-lateral.iq
@@ -0,0 +1,55 @@
+# 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 CALCITE_AS_ADMIN
+!set outputformat mysql
+
+# Checks whether CROSS JOIN LATERAL works
+SELECT SC.email EMAIL, TF.TYPE, TF.CODEVALUE
+FROM ENHANCED.SIMPLE_CUSTOMER SC
+CROSS JOIN LATERAL TABLE(AUX.CODES(SC.email)) TF limit 6;
++-----------------------+----------+------------------------------+
+| EMAIL                 | TYPE     | CODEVALUE                    |
++-----------------------+----------+------------------------------+
+| ftremblay@gmail.com   | BASE64   | ZnRyZW1ibGF5QGdtYWlsLmNvbQ== |
+| ftremblay@gmail.com   | HASHCODE | 1248316799                   |
+| leonekohler@surfeu.de | BASE64   | bGVvbmVrb2hsZXJAc3VyZmV1LmRl |
+| leonekohler@surfeu.de | HASHCODE | -1984160245                  |
+| luisg@embraer.com.br  | BASE64   | bHVpc2dAZW1icmFlci5jb20uYnI= |
+| luisg@embraer.com.br  | HASHCODE | 934160737                    |
++-----------------------+----------+------------------------------+
+(6 rows)
+
+!ok
+
+# [CALCITE-2446] Lateral joins not work when saved as custom views
+#
+# Checks whether CROSS JOIN LATERAL WORK WITH VIEW EXPANSION
+SELECT * FROM EXAMPLES.CODED_EMAILS;
++-----------------------+----------+------------------------------+
+| EMAIL                 | TYPE     | CODEVALUE                    |
++-----------------------+----------+------------------------------+
+| ftremblay@gmail.com   | BASE64   | ZnRyZW1ibGF5QGdtYWlsLmNvbQ== |
+| ftremblay@gmail.com   | HASHCODE | 1248316799                   |
+| leonekohler@surfeu.de | BASE64   | bGVvbmVrb2hsZXJAc3VyZmV1LmRl |
+| leonekohler@surfeu.de | HASHCODE | -1984160245                  |
+| luisg@embraer.com.br  | BASE64   | bHVpc2dAZW1icmFlci5jb20uYnI= |
+| luisg@embraer.com.br  | HASHCODE | 934160737                    |
++-----------------------+----------+------------------------------+
+(6 rows)
+
+!ok
+
+# End cross-join-lateral.iq