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 2016/11/11 08:07:48 UTC

[1/2] calcite git commit: [CALCITE-1384] Extension point for ALTER statements (Gabriel Reid)

Repository: calcite
Updated Branches:
  refs/heads/master aed0a9e90 -> f36584ab7


[CALCITE-1384] Extension point for ALTER statements (Gabriel Reid)

Add a parser extension point for parsing of ALTER statements.

The main content of this commit is the addition of an initial
test setup to allow testing parsing extension points within
Calcite itself. This involves the addition of test-specific
parser templates and building of a test-specific parser.

Fix running mvn clean install site, but ensuring that the
javacc generated test sources directory is excluded from the
source root when compiling (it's added as a source root by the
javacc plugin).

Also change the package of the parser extension testing classes
to make it more clear that they are only for testing (for added
clarity in the exclusions in the pom).

Close apache/calcite#322


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

Branch: refs/heads/master
Commit: fea541163cb609b4622972e7b25faa6ac9d60dd0
Parents: aed0a9e
Author: Gabriel Reid <ga...@ngdata.com>
Authored: Thu Nov 3 13:47:26 2016 +0100
Committer: Julian Hyde <jh...@apache.org>
Committed: Thu Nov 10 23:15:57 2016 -0800

----------------------------------------------------------------------
 core/pom.xml                                    | 48 +++++++++++--
 core/src/main/codegen/config.fmpp               |  6 ++
 core/src/main/codegen/templates/Parser.jj       | 55 +++++++++++----
 .../java/org/apache/calcite/sql/SqlAlter.java   | 60 ++++++++++++++++
 .../org/apache/calcite/sql/SqlSetOption.java    | 21 +-----
 core/src/test/codegen/config.fmpp               | 72 ++++++++++++++++++++
 .../codegen/includes/compoundIdentifier.ftl     | 34 +++++++++
 core/src/test/codegen/includes/parserImpls.ftl  | 37 ++++++++++
 .../calcite/sql/parser/SqlParserTest.java       | 28 ++++++--
 .../ExtensionSqlParserTest.java                 | 49 +++++++++++++
 .../SqlUploadJarNode.java                       | 63 +++++++++++++++++
 .../org/apache/calcite/test/CalciteSuite.java   |  2 +
 pom.xml                                         | 16 ++++-
 13 files changed, 450 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/fea54116/core/pom.xml
----------------------------------------------------------------------
diff --git a/core/pom.xml b/core/pom.xml
index 2176ef6..6fc6eb6 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -170,6 +170,18 @@ limitations under the License.
     <plugins>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-compiler-plugin</artifactId>
+        <configuration>
+          <source>1.7</source>
+          <target>1.7</target>
+          <excludes>
+            <exclude>org/apache/calcite/sql/parser/parserextensiontesting/*.java</exclude>
+          </excludes>
+          <generatedTestSourcesDirectory>${project.build.directory}/generated-test-sources/javacc</generatedTestSourcesDirectory>
+        </configuration>
+      </plugin>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-surefire-plugin</artifactId>
         <configuration>
           <includes>
@@ -195,6 +207,22 @@ limitations under the License.
               <isStatic>false</isStatic>
             </configuration>
           </execution>
+          <execution>
+            <id>javacc-test</id>
+            <phase>generate-test-sources</phase>
+            <goals>
+              <goal>javacc</goal>
+            </goals>
+            <configuration>
+              <sourceDirectory>${project.build.directory}/generated-test-sources/fmpp</sourceDirectory>
+              <outputDirectory>${project.build.directory}/generated-test-sources/javacc</outputDirectory>
+              <includes>
+                <include>**/Parser.jj</include>
+              </includes>
+              <lookAhead>2</lookAhead>
+              <isStatic>false</isStatic>
+            </configuration>
+          </execution>
         </executions>
       </plugin>
       <plugin>
@@ -444,18 +472,30 @@ limitations under the License.
           <plugin>
             <groupId>com.googlecode.fmpp-maven-plugin</groupId>
             <artifactId>fmpp-maven-plugin</artifactId>
-            <configuration>
-              <cfgFile>src/main/codegen/config.fmpp</cfgFile>
-              <templateDirectory>src/main/codegen/templates</templateDirectory>
-            </configuration>
             <executions>
               <execution>
+                <configuration>
+                  <cfgFile>src/main/codegen/config.fmpp</cfgFile>
+                  <templateDirectory>src/main/codegen/templates</templateDirectory>
+                </configuration>
                 <id>generate-fmpp-sources</id>
                 <phase>validate</phase>
                 <goals>
                   <goal>generate</goal>
                 </goals>
               </execution>
+              <execution>
+                <configuration>
+                  <cfgFile>src/test/codegen/config.fmpp</cfgFile>
+                  <templateDirectory>src/main/codegen/templates</templateDirectory>
+                  <outputDirectory>${project.build.directory}/generated-test-sources/fmpp</outputDirectory>
+                </configuration>
+                <id>generate-fmpp-test-sources</id>
+                <phase>validate</phase>
+                <goals>
+                  <goal>generate</goal>
+                </goals>
+              </execution>
             </executions>
           </plugin>
         </plugins>

http://git-wip-us.apache.org/repos/asf/calcite/blob/fea54116/core/src/main/codegen/config.fmpp
----------------------------------------------------------------------
diff --git a/core/src/main/codegen/config.fmpp b/core/src/main/codegen/config.fmpp
index 87ed18f..9f7a118 100644
--- a/core/src/main/codegen/config.fmpp
+++ b/core/src/main/codegen/config.fmpp
@@ -70,6 +70,12 @@ data: {
     dataTypeParserMethods: [
     ]
 
+    # List of methods for parsing extensions to ALTER SYSTEM calls.
+    # Each must accept arguments "(SqlParserPos pos, String scope)".
+    # Example: "SqlUploadJarNode"
+    alterStatementParserMethods: [
+    ]
+
     # List of files in @includes directory that have parser method
     # implementations for parsing custom SQL statements, literals or types
     # given as part of "statementParserMethods", "literalParserMethods" or

http://git-wip-us.apache.org/repos/asf/calcite/blob/fea54116/core/src/main/codegen/templates/Parser.jj
----------------------------------------------------------------------
diff --git a/core/src/main/codegen/templates/Parser.jj b/core/src/main/codegen/templates/Parser.jj
index aac7ab3..d6bb06a 100644
--- a/core/src/main/codegen/templates/Parser.jj
+++ b/core/src/main/codegen/templates/Parser.jj
@@ -41,6 +41,7 @@ import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.runtime.CalciteContextException;
 import org.apache.calcite.sql.JoinConditionType;
 import org.apache.calcite.sql.JoinType;
+import org.apache.calcite.sql.SqlAlter;
 import org.apache.calcite.sql.SqlBinaryOperator;
 import org.apache.calcite.sql.SqlCall;
 import org.apache.calcite.sql.SqlCharStringLiteral;
@@ -923,7 +924,9 @@ SqlNode SqlStmt() :
 }
 {
     (
-        stmt = SqlSetOption()
+        stmt = SqlSetOption(null, null)
+        |
+        stmt = SqlAlter()
         |
         stmt = OrderedQueryOrExpr(ExprContext.ACCEPT_QUERY)
         |
@@ -2965,22 +2968,16 @@ SqlCall SequenceExpression() :
 }
 
 /**
- * Parses an expression for setting or resetting an option in SQL, such as QUOTED_IDENTIFIERS,
- * or explain plan level (physical/logical).
+ * Parses "SET &lt;NAME&gt; = VALUE" or "RESET &lt;NAME&gt;", without a leading
+ * "ALTER &lt;SCOPE&gt;".
  */
-SqlSetOption SqlSetOption() :
+SqlSetOption SqlSetOption(SqlParserPos pos, String scope) :
 {
-    SqlParserPos pos = null;
-    String scope = null;
     SqlIdentifier name;
-    SqlNode val = null;
+    final SqlNode val;
 }
 {
     (
-        <ALTER> { pos = getPos(); }
-        scope = Scope()
-    )?
-    (
         <SET> {
             pos = pos == null ? getPos() : pos;
         }
@@ -2996,20 +2993,52 @@ SqlSetOption SqlSetOption() :
                 val = new SqlIdentifier(token.image.toUpperCase(), getPos());
             }
         )
+        {
+            return new SqlSetOption(pos.plus(getPos()), scope, name, val);
+        }
     |
         <RESET> {
             pos = pos == null ? getPos() : pos;
         }
         (
             name = CompoundIdentifier()
-            |
+        |
             <ALL> {
                 name = new SqlIdentifier(token.image.toUpperCase(), getPos());
             }
         )
+        {
+            return new SqlSetOption(pos.plus(getPos()), scope, name, null);
+        }
+    )
+}
+
+/**
+ * Parses an expression for setting or resetting an option in SQL, such as QUOTED_IDENTIFIERS,
+ * or explain plan level (physical/logical).
+ */
+SqlAlter SqlAlter() :
+{
+    final SqlParserPos pos;
+    final String scope;
+    final SqlAlter alterNode;
+}
+{
+    (
+        <ALTER> { pos = getPos(); }
+        scope = Scope()
+    )
+    (
+        alterNode = SqlSetOption(pos, scope)
+
+  <#-- additional literal parser methods are included here -->
+  <#list parser.alterStatementParserMethods as method>
+    |
+        alterNode = ${method}(pos, scope)
+  </#list>
     )
     {
-        return new SqlSetOption(pos.plus(getPos()), scope, name, val);
+        return alterNode;
     }
 }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/fea54116/core/src/main/java/org/apache/calcite/sql/SqlAlter.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlAlter.java b/core/src/main/java/org/apache/calcite/sql/SqlAlter.java
new file mode 100644
index 0000000..960c7ef
--- /dev/null
+++ b/core/src/main/java/org/apache/calcite/sql/SqlAlter.java
@@ -0,0 +1,60 @@
+/*
+ * 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.sql;
+
+import org.apache.calcite.sql.parser.SqlParserPos;
+
+/**
+ * Base class for an ALTER statements parse tree nodes. The portion of the
+ * statement covered by this class is "ALTER &lt;SCOPE&gt;. Subclasses handle
+ * whatever comes after the scope.
+ */
+public abstract class SqlAlter extends SqlCall {
+
+  /** Scope of the operation. Values "SYSTEM" and "SESSION" are typical. */
+  String scope;
+
+  public SqlAlter(SqlParserPos pos) {
+    this(pos, null);
+  }
+
+  public SqlAlter(SqlParserPos pos, String scope) {
+    super(pos);
+    this.scope = scope;
+  }
+
+  @Override public final void unparse(SqlWriter writer, int leftPrec, int rightPrec) {
+    if (scope != null) {
+      writer.keyword("ALTER");
+      writer.keyword(scope);
+    }
+    unparseAlterOperation(writer, leftPrec, rightPrec);
+  }
+
+  protected abstract void unparseAlterOperation(SqlWriter writer, int leftPrec, int rightPrec);
+
+  public String getScope() {
+    return scope;
+  }
+
+  public void setScope(String scope) {
+    this.scope = scope;
+  }
+
+}
+
+// End SqlAlter.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/fea54116/core/src/main/java/org/apache/calcite/sql/SqlSetOption.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlSetOption.java b/core/src/main/java/org/apache/calcite/sql/SqlSetOption.java
index 0d20d96..8538ffc 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlSetOption.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlSetOption.java
@@ -58,7 +58,7 @@ import java.util.List;
  * <li><code>ALTER SESSION RESET ALL</code></li>
  * </ul>
  */
-public class SqlSetOption extends SqlCall {
+public class SqlSetOption extends SqlAlter {
   public static final SqlSpecialOperator OPERATOR =
       new SqlSpecialOperator("SET_OPTION", SqlKind.SET_OPTION) {
         @Override public SqlCall createCall(SqlLiteral functionQualifier,
@@ -70,9 +70,6 @@ public class SqlSetOption extends SqlCall {
         }
       };
 
-  /** Scope of the assignment. Values "SYSTEM" and "SESSION" are typical. */
-  String scope;
-
   /** Name of the option as an {@link org.apache.calcite.sql.SqlIdentifier}
    * with one or more parts.*/
   SqlIdentifier name;
@@ -94,7 +91,7 @@ public class SqlSetOption extends SqlCall {
    */
   public SqlSetOption(SqlParserPos pos, String scope, SqlIdentifier name,
       SqlNode value) {
-    super(pos);
+    super(pos, scope);
     this.scope = scope;
     this.name = name;
     this.value = value;
@@ -141,11 +138,7 @@ public class SqlSetOption extends SqlCall {
     }
   }
 
-  @Override public void unparse(SqlWriter writer, int leftPrec, int rightPrec) {
-    if (scope != null) {
-      writer.keyword("ALTER");
-      writer.keyword(scope);
-    }
+  @Override protected void unparseAlterOperation(SqlWriter writer, int leftPrec, int rightPrec) {
     if (value != null) {
       writer.keyword("SET");
     } else {
@@ -174,14 +167,6 @@ public class SqlSetOption extends SqlCall {
     this.name = name;
   }
 
-  public String getScope() {
-    return scope;
-  }
-
-  public void setScope(String scope) {
-    this.scope = scope;
-  }
-
   public SqlNode getValue() {
     return value;
   }

http://git-wip-us.apache.org/repos/asf/calcite/blob/fea54116/core/src/test/codegen/config.fmpp
----------------------------------------------------------------------
diff --git a/core/src/test/codegen/config.fmpp b/core/src/test/codegen/config.fmpp
new file mode 100644
index 0000000..7b8b8e2
--- /dev/null
+++ b/core/src/test/codegen/config.fmpp
@@ -0,0 +1,72 @@
+# 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.
+
+data: {
+    parser: {
+      # Generated parser implementation class package and name
+      package: "org.apache.calcite.sql.parser.parserextensiontesting",
+      class: "ExtensionSqlParserImpl",
+
+      # List of import statements.
+      imports: [
+        "org.apache.calcite.sql.parser.parserextensiontesting.SqlUploadJarNode"
+      ]
+
+      # List of keywords.
+      keywords: [
+        "UPLOAD"
+        "JAR"
+      ]
+
+      # List of keywords from "keywords" section that are not reserved.
+      nonReservedKeywords: [
+      ]
+
+      # List of methods for parsing custom SQL statements.
+      statementParserMethods: [
+      ]
+
+      # List of methods for parsing custom literals.
+      # Example: ParseJsonLiteral().
+      literalParserMethods: [
+      ]
+
+      # List of methods for parsing custom data types.
+      dataTypeParserMethods: [
+      ]
+
+      # List of methods for parsing extensions to ALTER SYSTEM calls.
+      # Each must accept arguments "(SqlParserPos pos, String scope)".
+      alterStatementParserMethods: [
+        "SqlUploadJarNode"
+      ]
+
+      # List of files in @includes directory that have parser method
+      # implementations for custom SQL statements, literals or types
+      # given as part of "statementParserMethods", "literalParserMethods" or
+      # "dataTypeParserMethods".
+      implementationFiles: [
+        "parserImpls.ftl"
+      ]
+
+      includeCompoundIdentifier: true
+      includeBraces: true
+      includeAdditionalDeclarations: false
+
+    }
+}
+freemarkerLinks: {
+    includes: includes/
+}

http://git-wip-us.apache.org/repos/asf/calcite/blob/fea54116/core/src/test/codegen/includes/compoundIdentifier.ftl
----------------------------------------------------------------------
diff --git a/core/src/test/codegen/includes/compoundIdentifier.ftl b/core/src/test/codegen/includes/compoundIdentifier.ftl
new file mode 100644
index 0000000..70db3c2
--- /dev/null
+++ b/core/src/test/codegen/includes/compoundIdentifier.ftl
@@ -0,0 +1,34 @@
+<#--
+// 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.
+-->
+
+<#--
+  Add implementations of additional parser statements, literals or
+  data types.
+
+  Example of SqlShowTables() implementation:
+  SqlNode SqlShowTables()
+  {
+    ...local variables...
+  }
+  {
+    <SHOW> <TABLES>
+    ...
+    {
+      return SqlShowTables(...)
+    }
+  }
+-->

http://git-wip-us.apache.org/repos/asf/calcite/blob/fea54116/core/src/test/codegen/includes/parserImpls.ftl
----------------------------------------------------------------------
diff --git a/core/src/test/codegen/includes/parserImpls.ftl b/core/src/test/codegen/includes/parserImpls.ftl
new file mode 100644
index 0000000..2305fc1
--- /dev/null
+++ b/core/src/test/codegen/includes/parserImpls.ftl
@@ -0,0 +1,37 @@
+<#--
+// 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.
+-->
+
+
+SqlAlter SqlUploadJarNode(SqlParserPos pos, String scope) :
+{
+    SqlNode jarPath;
+    final List<SqlNode> jarPathsList;
+}
+{
+    <UPLOAD> <JAR>
+    jarPath = StringLiteral() {
+        jarPathsList = startList(jarPath);
+    }
+    (
+        <COMMA> jarPath = StringLiteral() {
+            jarPathsList.add(jarPath);
+        }
+    )*
+    {
+        return new SqlUploadJarNode(pos.plus(getPos()), scope, jarPathsList);
+    }
+}

http://git-wip-us.apache.org/repos/asf/calcite/blob/fea54116/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
index b5ce24b..7144383 100644
--- a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
@@ -22,6 +22,7 @@ import org.apache.calcite.sql.SqlDialect;
 import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.SqlSetOption;
+import org.apache.calcite.sql.parser.impl.SqlParserImpl;
 import org.apache.calcite.sql.pretty.SqlPrettyWriter;
 import org.apache.calcite.sql.validate.SqlConformance;
 import org.apache.calcite.sql.validate.SqlConformanceEnum;
@@ -64,10 +65,15 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
 
 /**
  * A <code>SqlParserTest</code> is a unit-test for
  * {@link SqlParser the SQL parser}.
+ *
+ * <p>To reuse this test for an extension parser, implement the
+ * {@link #parserImplFactory()} method to return the extension parser
+ * implementation.
  */
 public class SqlParserTest {
   //~ Static fields/initializers ---------------------------------------------
@@ -543,9 +549,18 @@ public class SqlParserTest {
     return new Sql(sql);
   }
 
-  private SqlParser getSqlParser(String sql) {
+  /**
+   * Implementors of custom parsing logic who want to reuse this test should
+   * override this method with the factory for their extension parser.
+   */
+  protected SqlParserImplFactory parserImplFactory() {
+    return SqlParserImpl.FACTORY;
+  }
+
+  protected SqlParser getSqlParser(String sql) {
     return SqlParser.create(sql,
         SqlParser.configBuilder()
+            .setParserFactory(parserImplFactory())
             .setQuoting(quoting)
             .setUnquotedCasing(unquotedCasing)
             .setQuotedCasing(quotedCasing)
@@ -6681,6 +6696,7 @@ public class SqlParserTest {
    * non-reserved keyword list in the parser.
    */
   @Test public void testNoUnintendedNewReservedKeywords() {
+    assumeTrue("don't run this test for sub-classes", isNotSubclass());
     final SqlAbstractParserImpl.Metadata metadata =
         getSqlParser("").getMetadata();
 
@@ -6706,6 +6722,7 @@ public class SqlParserTest {
   /** Generates a copy of {@code reference.md} with the current set of key
    * words. Fails if the copy is different from the original. */
   @Test public void testGenerateKeyWords() throws IOException {
+    assumeTrue("don't run this test for sub-classes", isNotSubclass());
     // inUrl = "file:/home/x/calcite/core/target/test-classes/hsqldb-model.json"
     String path = "hsqldb-model.json";
     final URL inUrl = SqlParserTest.class.getResource("/" + path);
@@ -6908,8 +6925,7 @@ public class SqlParserTest {
   }
 
   @Test public void testSqlOptions() throws SqlParseException {
-    SqlNode node =
-        SqlParser.create("alter system set schema = true").parseStmt();
+    SqlNode node = getSqlParser("alter system set schema = true").parseStmt();
     SqlSetOption opt = (SqlSetOption) node;
     assertThat(opt.getScope(), equalTo("SYSTEM"));
     SqlPrettyWriter writer = new SqlPrettyWriter(SqlDialect.CALCITE);
@@ -6941,7 +6957,7 @@ public class SqlParserTest {
         .ok("SET `APPROX` = -12.3450")
         .node(isDdl());
 
-    node = SqlParser.create("reset schema").parseStmt();
+    node = getSqlParser("reset schema").parseStmt();
     opt = (SqlSetOption) node;
     assertThat(opt.getScope(), equalTo(null));
     writer = new SqlPrettyWriter(SqlDialect.CALCITE);
@@ -7115,6 +7131,10 @@ public class SqlParserTest {
     }
   }
 
+  private boolean isNotSubclass() {
+    return this.getClass().equals(SqlParserTest.class);
+  }
+
   /**
    * Implementation of {@link Tester} which makes sure that the results of
    * unparsing a query are consistent with the original query.

http://git-wip-us.apache.org/repos/asf/calcite/blob/fea54116/core/src/test/java/org/apache/calcite/sql/parser/parserextensiontesting/ExtensionSqlParserTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/sql/parser/parserextensiontesting/ExtensionSqlParserTest.java b/core/src/test/java/org/apache/calcite/sql/parser/parserextensiontesting/ExtensionSqlParserTest.java
new file mode 100644
index 0000000..51e24bd
--- /dev/null
+++ b/core/src/test/java/org/apache/calcite/sql/parser/parserextensiontesting/ExtensionSqlParserTest.java
@@ -0,0 +1,49 @@
+/*
+ * 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.sql.parser.parserextensiontesting;
+
+import org.apache.calcite.sql.parser.SqlParseException;
+import org.apache.calcite.sql.parser.SqlParserImplFactory;
+import org.apache.calcite.sql.parser.SqlParserTest;
+
+import org.junit.Test;
+
+/**
+ * Testing for extension functionality of the base SQL parser impl.
+ *
+ * <p>This test runs all test cases of the base {@link SqlParserTest}, as well
+ * as verifying specific extension points.
+ */
+public class ExtensionSqlParserTest extends SqlParserTest {
+
+  @Override protected SqlParserImplFactory parserImplFactory() {
+    return ExtensionSqlParserImpl.FACTORY;
+  }
+
+  @Test public void testAlterSystemExtension() throws SqlParseException {
+    check("alter system upload jar '/path/to/jar'",
+      "ALTER SYSTEM UPLOAD JAR '/path/to/jar'");
+  }
+
+  @Test public void testAlterSystemExtensionWithoutAlter() throws SqlParseException {
+    // We need to include the scope for custom alter operations
+    checkFails("^upload^ jar '/path/to/jar'",
+      "(?s).*Encountered \"upload\" at .*");
+  }
+}
+
+// End ExtensionSqlParserTest.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/fea54116/core/src/test/java/org/apache/calcite/sql/parser/parserextensiontesting/SqlUploadJarNode.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/sql/parser/parserextensiontesting/SqlUploadJarNode.java b/core/src/test/java/org/apache/calcite/sql/parser/parserextensiontesting/SqlUploadJarNode.java
new file mode 100644
index 0000000..78184d0
--- /dev/null
+++ b/core/src/test/java/org/apache/calcite/sql/parser/parserextensiontesting/SqlUploadJarNode.java
@@ -0,0 +1,63 @@
+/*
+ * 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.sql.parser.parserextensiontesting;
+
+import org.apache.calcite.sql.SqlAlter;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+
+import java.util.List;
+
+/**
+ * Simple test example of a custom alter system call.
+ */
+public class SqlUploadJarNode extends SqlAlter {
+  public static final SqlOperator OPERATOR =
+      new SqlSpecialOperator("UPLOAD JAR", SqlKind.OTHER_DDL);
+
+  private final List<SqlNode> jarPaths;
+
+  /** Creates a SqlUploadJarNode. */
+  public SqlUploadJarNode(SqlParserPos pos, String scope, List<SqlNode> jarPaths) {
+    super(pos, scope);
+    this.jarPaths = jarPaths;
+  }
+
+  @Override public SqlOperator getOperator() {
+    return OPERATOR;
+  }
+
+  @Override public List<SqlNode> getOperandList() {
+    return jarPaths;
+  }
+
+  @Override protected void unparseAlterOperation(SqlWriter writer, int leftPrec, int rightPrec) {
+    writer.keyword("UPLOAD");
+    writer.keyword("JAR");
+    SqlWriter.Frame frame = writer.startList("", "");
+    for (SqlNode jarPath : jarPaths) {
+      jarPath.unparse(writer, leftPrec, rightPrec);
+    }
+    writer.endList(frame);
+  }
+}
+
+// End SqlUploadJarNode.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/fea54116/core/src/test/java/org/apache/calcite/test/CalciteSuite.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/CalciteSuite.java b/core/src/test/java/org/apache/calcite/test/CalciteSuite.java
index dc59a8e..df0167d 100644
--- a/core/src/test/java/org/apache/calcite/test/CalciteSuite.java
+++ b/core/src/test/java/org/apache/calcite/test/CalciteSuite.java
@@ -38,6 +38,7 @@ import org.apache.calcite.runtime.EnumerablesTest;
 import org.apache.calcite.sql.SqlSetOptionOperatorTest;
 import org.apache.calcite.sql.parser.SqlParserTest;
 import org.apache.calcite.sql.parser.SqlUnParserTest;
+import org.apache.calcite.sql.parser.parserextensiontesting.ExtensionSqlParserTest;
 import org.apache.calcite.sql.test.SqlAdvisorTest;
 import org.apache.calcite.sql.test.SqlOperatorTest;
 import org.apache.calcite.sql.test.SqlPrettyWriterTest;
@@ -112,6 +113,7 @@ import org.junit.runners.Suite;
     // medium tests (above 0.1s)
     SqlParserTest.class,
     SqlUnParserTest.class,
+    ExtensionSqlParserTest.class,
     SqlSetOptionOperatorTest.class,
     SqlPrettyWriterTest.class,
     SqlValidatorTest.class,

http://git-wip-us.apache.org/repos/asf/calcite/blob/fea54116/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 495eb70..8cddb3c 100644
--- a/pom.xml
+++ b/pom.xml
@@ -553,7 +553,7 @@ limitations under the License.
           <links>
             <link>https://docs.oracle.com/javase/8/docs/api/</link>
           </links>
-          <excludePackageNames>org.apache.calcite.benchmarks.generated,org.apache.calcite.sql.parser.impl,org.apache.calcite.piglet.parser,org.openjdk.jmh</excludePackageNames>
+          <excludePackageNames>org.apache.calcite.benchmarks.generated,org.apache.calcite.sql.parser.impl,org.apache.calcite.sql.parser.parserextensiontesting,org.apache.calcite.piglet.parser,org.openjdk.jmh</excludePackageNames>
           <tags>
             <tag>
               <name>sql.92</name>
@@ -627,6 +627,18 @@ limitations under the License.
               </resources>
             </configuration>
           </execution>
+          <execution>
+            <id>add-test-sources</id>
+            <phase>generate-test-sources</phase>
+            <goals>
+              <goal>add-test-source</goal>
+            </goals>
+            <configuration>
+              <sources>
+                <source>${project.build.directory}/generated-test-sources/javacc</source>
+              </sources>
+            </configuration>
+          </execution>
         </executions>
       </plugin>
     </plugins>
@@ -729,7 +741,7 @@ limitations under the License.
           <links>
             <link>https://docs.oracle.com/javase/8/docs/api/</link>
           </links>
-          <excludePackageNames>org.apache.calcite.benchmarks.generated,org.apache.calcite.sql.parser.impl,org.apache.calcite.piglet.parser,org.openjdk.jmh</excludePackageNames>
+          <excludePackageNames>org.apache.calcite.benchmarks.generated,org.apache.calcite.sql.parser.impl,org.apache.calcite.sql.parser.parserextensiontesting,org.apache.calcite.piglet.parser,org.openjdk.jmh</excludePackageNames>
           <tags>
             <tag>
               <name>sql.92</name>


[2/2] calcite git commit: [CALCITE-1488] ValuesReduceRule should ignore empty Values

Posted by jh...@apache.org.
[CALCITE-1488] ValuesReduceRule should ignore empty Values


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

Branch: refs/heads/master
Commit: f36584ab7718496de3430aa80b97f4ea98c9b308
Parents: fea5411
Author: Julian Hyde <jh...@apache.org>
Authored: Thu Nov 10 23:02:15 2016 -0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Thu Nov 10 23:18:07 2016 -0800

----------------------------------------------------------------------
 .../org/apache/calcite/rel/core/Values.java     | 14 ++++++
 .../calcite/rel/rules/PruneEmptyRules.java      |  2 +-
 .../calcite/rel/rules/ValuesReduceRule.java     | 13 ++++--
 .../apache/calcite/test/RelOptRulesTest.java    | 32 ++++++++++++++
 .../org/apache/calcite/test/RelOptRulesTest.xml | 46 ++++++++++++++++++++
 5 files changed, 102 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/f36584ab/core/src/main/java/org/apache/calcite/rel/core/Values.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/core/Values.java b/core/src/main/java/org/apache/calcite/rel/core/Values.java
index bf99fae..f9c9880 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/Values.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/Values.java
@@ -71,6 +71,20 @@ public abstract class Values extends AbstractRelNode {
         }
       };
 
+  /** Predicate, to be used when defining an operand of a {@link RelOptRule},
+   * that returns true if a Values contains one or more tuples.
+   *
+   * <p>This is the conventional way to represent an empty relational
+   * expression. There are several rules that recognize empty relational
+   * expressions and prune away that section of the tree.
+   */
+  public static final Predicate<? super Values> IS_NOT_EMPTY =
+      new Predicate<Values>() {
+        public boolean apply(Values values) {
+          return !values.getTuples().isEmpty();
+        }
+      };
+
   //~ Instance fields --------------------------------------------------------
 
   public final ImmutableList<ImmutableList<RexLiteral>> tuples;

http://git-wip-us.apache.org/repos/asf/calcite/blob/f36584ab/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java b/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java
index acd1a1c..094c290 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java
@@ -336,7 +336,7 @@ public abstract class PruneEmptyRules {
           description);
     }
 
-    /** Creatse a RemoveEmptySingleRule. */
+    /** Creates a RemoveEmptySingleRule. */
     public <R extends SingleRel> RemoveEmptySingleRule(Class<R> clazz,
         Predicate<R> predicate, RelBuilderFactory relBuilderFactory,
         String description) {

http://git-wip-us.apache.org/repos/asf/calcite/blob/f36584ab/core/src/main/java/org/apache/calcite/rel/rules/ValuesReduceRule.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/ValuesReduceRule.java b/core/src/main/java/org/apache/calcite/rel/rules/ValuesReduceRule.java
index 1f5c699..7a194c0 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/ValuesReduceRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/ValuesReduceRule.java
@@ -21,6 +21,7 @@ import org.apache.calcite.plan.RelOptRule;
 import org.apache.calcite.plan.RelOptRuleCall;
 import org.apache.calcite.plan.RelOptRuleOperand;
 import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Values;
 import org.apache.calcite.rel.logical.LogicalFilter;
 import org.apache.calcite.rel.logical.LogicalProject;
 import org.apache.calcite.rel.logical.LogicalValues;
@@ -56,6 +57,9 @@ import java.util.List;
  * <p>becomes</p>
  *
  * <blockquote><code>select x from (values (-2), (-4))</code></blockquote>
+ *
+ * <p>Ignores an empty {@code Values}; this is better dealt with by
+ * {@link PruneEmptyRules}.
  */
 public abstract class ValuesReduceRule extends RelOptRule {
   //~ Static fields/initializers ---------------------------------------------
@@ -69,7 +73,7 @@ public abstract class ValuesReduceRule extends RelOptRule {
   public static final ValuesReduceRule FILTER_INSTANCE =
       new ValuesReduceRule(
           operand(LogicalFilter.class,
-              operand(LogicalValues.class, none())),
+              operand(LogicalValues.class, null, Values.IS_NOT_EMPTY, none())),
           "ValuesReduceRule(Filter)") {
         public void onMatch(RelOptRuleCall call) {
           LogicalFilter filter = call.rel(0);
@@ -85,7 +89,7 @@ public abstract class ValuesReduceRule extends RelOptRule {
   public static final ValuesReduceRule PROJECT_INSTANCE =
       new ValuesReduceRule(
           operand(LogicalProject.class,
-              operand(LogicalValues.class, none())),
+              operand(LogicalValues.class, null, Values.IS_NOT_EMPTY, none())),
           "ValuesReduceRule(Project)") {
         public void onMatch(RelOptRuleCall call) {
           LogicalProject project = call.rel(0);
@@ -102,7 +106,8 @@ public abstract class ValuesReduceRule extends RelOptRule {
       new ValuesReduceRule(
           operand(LogicalProject.class,
               operand(LogicalFilter.class,
-                  operand(LogicalValues.class, none()))),
+                  operand(LogicalValues.class, null, Values.IS_NOT_EMPTY,
+                      none()))),
           "ValuesReduceRule(Project-Filter)") {
         public void onMatch(RelOptRuleCall call) {
           LogicalProject project = call.rel(0);
@@ -145,7 +150,7 @@ public abstract class ValuesReduceRule extends RelOptRule {
     RexBuilder rexBuilder = values.getCluster().getRexBuilder();
 
     // Find reducible expressions.
-    List<RexNode> reducibleExps = new ArrayList<RexNode>();
+    final List<RexNode> reducibleExps = new ArrayList<>();
     final MyRexShuttle shuttle = new MyRexShuttle();
     for (final List<RexLiteral> literalList : values.getTuples()) {
       shuttle.literalList = literalList;

http://git-wip-us.apache.org/repos/asf/calcite/blob/f36584ab/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index ac96e14..8ca9130 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -1528,6 +1528,38 @@ public class RelOptRulesTest extends RelOptTestBase {
             + "where x + y > 30");
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-1488">[CALCITE-1488]
+   * ValuesReduceRule should ignore empty Values</a>. */
+  @Test public void testEmptyProject() throws Exception {
+    HepProgram program = new HepProgramBuilder()
+        .addRuleInstance(ValuesReduceRule.PROJECT_FILTER_INSTANCE)
+        .addRuleInstance(ValuesReduceRule.FILTER_INSTANCE)
+        .addRuleInstance(ValuesReduceRule.PROJECT_INSTANCE)
+        .build();
+
+    final String sql = "select z + x from (\n"
+        + "  select x + y as z, x from (\n"
+        + "    select * from (values (10, 1), (30, 3)) as t (x, y)\n"
+        + "    where x + y > 50))";
+    sql(sql).with(program).check();
+  }
+
+  /** Same query as {@link #testEmptyProject()}, and {@link PruneEmptyRules}
+   * is able to do the job that {@link ValuesReduceRule} cannot do. */
+  @Test public void testEmptyProject2() throws Exception {
+    HepProgram program = new HepProgramBuilder()
+        .addRuleInstance(ValuesReduceRule.FILTER_INSTANCE)
+        .addRuleInstance(PruneEmptyRules.PROJECT_INSTANCE)
+        .build();
+
+    final String sql = "select z + x from (\n"
+        + "  select x + y as z, x from (\n"
+        + "    select * from (values (10, 1), (30, 3)) as t (x, y)\n"
+        + "    where x + y > 50))";
+    sql(sql).with(program).check();
+  }
+
   @Test public void testEmptyIntersect() throws Exception {
     HepProgram program = new HepProgramBuilder()
         .addRuleInstance(ValuesReduceRule.PROJECT_FILTER_INSTANCE)

http://git-wip-us.apache.org/repos/asf/calcite/blob/f36584ab/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
----------------------------------------------------------------------
diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
index 11c5a98..92fa0f7 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -809,6 +809,52 @@ LogicalMinus(all=[false])
 ]]>
         </Resource>
     </TestCase>
+    <TestCase name="testEmptyProject">
+        <Resource name="sql">
+            <![CDATA[select z + x from (
+  select x + y as z, x from (
+    select * from (values (10, 1), (30, 3)) as t (x, y)
+    where x + y > 50))]]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+LogicalProject(EXPR$0=[+($0, $1)])
+  LogicalProject(Z=[+($0, $1)], X=[$0])
+    LogicalProject(X=[$0], Y=[$1])
+      LogicalFilter(condition=[>(+($0, $1), 50)])
+        LogicalValues(tuples=[[{ 10, 1 }, { 30, 3 }]])
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+LogicalProject(EXPR$0=[+($0, $1)])
+  LogicalProject(Z=[+($0, $1)], X=[$0])
+    LogicalValues(tuples=[[]])
+]]>
+        </Resource>
+    </TestCase>
+    <TestCase name="testEmptyProject2">
+        <Resource name="sql">
+            <![CDATA[select z + x from (
+  select x + y as z, x from (
+    select * from (values (10, 1), (30, 3)) as t (x, y)
+    where x + y > 50))]]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+LogicalProject(EXPR$0=[+($0, $1)])
+  LogicalProject(Z=[+($0, $1)], X=[$0])
+    LogicalProject(X=[$0], Y=[$1])
+      LogicalFilter(condition=[>(+($0, $1), 50)])
+        LogicalValues(tuples=[[{ 10, 1 }, { 30, 3 }]])
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+LogicalValues(tuples=[[]])
+]]>
+        </Resource>
+    </TestCase>
     <TestCase name="testReduceValuesUnderFilter">
         <Resource name="sql">
             <![CDATA[select a, b from (values (10, 'x'), (20, 'y')) as t(a, b) where a < 15]]>