From ccd5797c4b3631af93db5e29d202dc403e05c775 Mon Sep 17 00:00:00 2001 From: Gabriel Reid Date: Thu, 3 Nov 2016 13:47:26 +0100 Subject: [PATCH] CALCITE-1384 Extension point for ALTER statements 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. --- core/pom.xml | 46 ++++++++++-- core/src/main/codegen/config.fmpp | 4 ++ core/src/main/codegen/templates/Parser.jj | 53 +++++++++++--- .../java/org/apache/calcite/sql/SqlAlter.java | 60 ++++++++++++++++ .../org/apache/calcite/sql/SqlSetOption.java | 21 +----- core/src/test/codegen/config.fmpp | 71 +++++++++++++++++++ .../codegen/includes/compoundIdentifier.ftl | 34 +++++++++ .../src/test/codegen/includes/parserImpls.ftl | 38 ++++++++++ .../calcite/sql/parser/SqlParserTest.java | 28 ++++++-- .../extension/ExtensionSqlParserTest.java | 51 +++++++++++++ .../parser/extension/SqlUploadJarNode.java | 61 ++++++++++++++++ pom.xml | 12 ++++ 12 files changed, 445 insertions(+), 34 deletions(-) create mode 100644 core/src/main/java/org/apache/calcite/sql/SqlAlter.java create mode 100644 core/src/test/codegen/config.fmpp create mode 100644 core/src/test/codegen/includes/compoundIdentifier.ftl create mode 100644 core/src/test/codegen/includes/parserImpls.ftl create mode 100644 core/src/test/java/org/apache/calcite/sql/parser/extension/ExtensionSqlParserTest.java create mode 100644 core/src/test/java/org/apache/calcite/sql/parser/extension/SqlUploadJarNode.java diff --git a/core/pom.xml b/core/pom.xml index 2176ef6c9b90..eccd0d7c0bd4 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -168,6 +168,16 @@ limitations under the License. + + org.apache.maven.plugins + maven-compiler-plugin + + 1.7 + 1.7 + ${project.build.directory}/generated-test-sources/javacc + ${project.build.directory}/generated-test-sources/javacc + + org.apache.maven.plugins maven-surefire-plugin @@ -195,6 +205,22 @@ limitations under the License. false + + javacc-test + generate-test-sources + + javacc + + + ${project.build.directory}/generated-test-sources/fmpp + ${project.build.directory}/generated-test-sources/javacc + + **/Parser.jj + + 2 + false + + @@ -444,18 +470,30 @@ limitations under the License. com.googlecode.fmpp-maven-plugin fmpp-maven-plugin - - src/main/codegen/config.fmpp - src/main/codegen/templates - + + src/main/codegen/config.fmpp + src/main/codegen/templates + generate-fmpp-sources validate generate + + + src/test/codegen/config.fmpp + src/main/codegen/templates + ${project.build.directory}/generated-test-sources/fmpp + + generate-fmpp-test-sources + validate + + generate + + diff --git a/core/src/main/codegen/config.fmpp b/core/src/main/codegen/config.fmpp index 87ed18f28302..a1a1071688cd 100644 --- a/core/src/main/codegen/config.fmpp +++ b/core/src/main/codegen/config.fmpp @@ -70,6 +70,10 @@ data: { dataTypeParserMethods: [ ] + # List of methods for parsing extensions to ALTER SYSTEM calls + 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 diff --git a/core/src/main/codegen/templates/Parser.jj b/core/src/main/codegen/templates/Parser.jj index aac7ab3003c3..1ba1d1eb843e 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; @@ -925,6 +926,8 @@ SqlNode SqlStmt() : ( stmt = SqlSetOption() | + stmt = SqlAlter() + | stmt = OrderedQueryOrExpr(ExprContext.ACCEPT_QUERY) | stmt = SqlExplain() @@ -2965,21 +2968,17 @@ 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 = VALUE" or "RESET ", without a leading + * "ALTER ". */ SqlSetOption SqlSetOption() : { SqlParserPos pos = null; - String scope = null; SqlIdentifier name; SqlNode val = null; + SqlSetOption setOption = null; } { - ( - { pos = getPos(); } - scope = Scope() - )? ( { pos = pos == null ? getPos() : pos; @@ -2996,6 +2995,9 @@ SqlSetOption SqlSetOption() : val = new SqlIdentifier(token.image.toUpperCase(), getPos()); } ) + { + setOption = new SqlSetOption(pos.plus(getPos()), null, name, val); + } | { pos = pos == null ? getPos() : pos; @@ -3007,9 +3009,44 @@ SqlSetOption SqlSetOption() : name = new SqlIdentifier(token.image.toUpperCase(), getPos()); } ) + { + setOption = new SqlSetOption(pos.plus(getPos()), null, name, val); + } + ) + { + return setOption; + } +} + +/** + * Parses an expression for setting or resetting an option in SQL, such as QUOTED_IDENTIFIERS, + * or explain plan level (physical/logical). + */ +SqlAlter SqlAlter() : +{ + SqlParserPos pos = null; + String scope = null; + SqlIdentifier name; + SqlNode val = null; + SqlAlter alterNode = null; +} +{ + ( + { pos = getPos(); } + scope = Scope() + ) + ( + alterNode = SqlSetOption() + + <#-- additional literal parser methods are included here --> + <#list parser.alterStatementParserMethods as method> + | + alterNode = ${method} + ) { - return new SqlSetOption(pos.plus(getPos()), scope, name, val); + alterNode.setScope(scope); + return alterNode; } } 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 000000000000..960c7ef39809 --- /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 <SCOPE>. 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 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 0d20d96a2269..8538ffc3172e 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 @@ *
  • ALTER SESSION RESET ALL
  • * */ -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 SqlSetOption(SqlParserPos pos, String scope, SqlIdentifier name, } } - @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 void setName(SqlIdentifier name) { this.name = name; } - public String getScope() { - return scope; - } - - public void setScope(String scope) { - this.scope = scope; - } - public SqlNode getValue() { return value; } diff --git a/core/src/test/codegen/config.fmpp b/core/src/test/codegen/config.fmpp new file mode 100644 index 000000000000..e3e2b64fc0f1 --- /dev/null +++ b/core/src/test/codegen/config.fmpp @@ -0,0 +1,71 @@ +# 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.extension", + class: "ExtensionSqlParserImpl", + + # List of import statements. + imports: [ + "org.apache.calcite.sql.parser.extension.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 + 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/ +} diff --git a/core/src/test/codegen/includes/compoundIdentifier.ftl b/core/src/test/codegen/includes/compoundIdentifier.ftl new file mode 100644 index 000000000000..70db3c2ee3bd --- /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... + } + { + + ... + { + return SqlShowTables(...) + } + } +--> diff --git a/core/src/test/codegen/includes/parserImpls.ftl b/core/src/test/codegen/includes/parserImpls.ftl new file mode 100644 index 000000000000..fa40024c6db6 --- /dev/null +++ b/core/src/test/codegen/includes/parserImpls.ftl @@ -0,0 +1,38 @@ +<#-- +// 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; + SqlNode jarPath; + List jarPathsList; +} +{ + { pos = getPos(); } + jarPath = StringLiteral() { + jarPathsList = startList(jarPath); + } + ( + jarPath = StringLiteral() { + jarPathsList.add(jarPath); + } + ) * + { + return new SqlUploadJarNode(pos.plus(getPos()), jarPathsList); + } +} 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 b5ce24bb6f38..1011a2dc5636 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.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.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; /** * A SqlParserTest is a unit-test for * {@link SqlParser the SQL parser}. + * + * 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 @@ protected Sql sql(String sql) { 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 void subTestIntervalSecondFailsValidation() { * non-reserved keyword list in the parser. */ @Test public void testNoUnintendedNewReservedKeywords() { + assumeTrue(isNotSubclass()); // This shouldn't run for subclasses final SqlAbstractParserImpl.Metadata metadata = getSqlParser("").getMetadata(); @@ -6706,6 +6722,7 @@ public void subTestIntervalSecondFailsValidation() { /** 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(isNotSubclass()); // This shouldn't run for subclasses // 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 void subTestIntervalSecondFailsValidation() { } @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 void subTestIntervalSecondFailsValidation() { .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 void checkExpFails( } } + 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. diff --git a/core/src/test/java/org/apache/calcite/sql/parser/extension/ExtensionSqlParserTest.java b/core/src/test/java/org/apache/calcite/sql/parser/extension/ExtensionSqlParserTest.java new file mode 100644 index 000000000000..054721663240 --- /dev/null +++ b/core/src/test/java/org/apache/calcite/sql/parser/extension/ExtensionSqlParserTest.java @@ -0,0 +1,51 @@ +/* + * 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.extension; + +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. + * + * 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 diff --git a/core/src/test/java/org/apache/calcite/sql/parser/extension/SqlUploadJarNode.java b/core/src/test/java/org/apache/calcite/sql/parser/extension/SqlUploadJarNode.java new file mode 100644 index 000000000000..582b4eb968c6 --- /dev/null +++ b/core/src/test/java/org/apache/calcite/sql/parser/extension/SqlUploadJarNode.java @@ -0,0 +1,61 @@ +/* + * 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.extension; + +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 jarPaths; + + public SqlUploadJarNode(SqlParserPos pos, List jarPaths) { + super(pos); + this.jarPaths = jarPaths; + } + + @Override public SqlOperator getOperator() { + return OPERATOR; + } + + @Override public List 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 diff --git a/pom.xml b/pom.xml index 45d1fdb9b650..0f49d09154a6 100644 --- a/pom.xml +++ b/pom.xml @@ -633,6 +633,18 @@ limitations under the License. + + add-test-sources + generate-test-sources + + add-test-source + + + + ${project.build.directory}/generated-test-sources/javacc + + +