Repository: calcite Updated Branches: refs/heads/master d0bdec4f6 -> be5404713
[CALCITE-2662] In Planner, allow parsing a stream (Reader) instead of a String (Enrico Olivelli) Add SourceStringReader, a simple sub-class of StringReader that remembers the original string. This allows us to use Reader for most parser methods, knowing we can find the string if the parser was invoked on a string, but null if it was invoked on an infinite reader. Close apache/calcite#906 Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/03c88b69 Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/03c88b69 Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/03c88b69 Branch: refs/heads/master Commit: 03c88b69c27ce7df22eee889503c70e6fe6bb016 Parents: d0bdec4 Author: Enrico Olivelli <[email protected]> Authored: Thu Nov 8 08:10:18 2018 +0100 Committer: Julian Hyde <[email protected]> Committed: Mon Dec 3 10:56:03 2018 -0800 ---------------------------------------------------------------------- core/src/main/codegen/templates/Parser.jj | 11 ++++- .../org/apache/calcite/prepare/PlannerImpl.java | 5 +- .../apache/calcite/sql/parser/SqlParser.java | 49 ++++++++++++++------ .../java/org/apache/calcite/tools/Planner.java | 17 ++++++- .../apache/calcite/util/SourceStringReader.java | 46 ++++++++++++++++++ .../calcite/sql/parser/SqlParserTest.java | 18 ++++++- .../apache/calcite/sql/test/SqlTestFactory.java | 3 +- .../apache/calcite/test/SqlValidatorTest.java | 47 +++++++++++++++++++ 8 files changed, 174 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/03c88b69/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 b137d6f..cf6b3f2 100644 --- a/core/src/main/codegen/templates/Parser.jj +++ b/core/src/main/codegen/templates/Parser.jj @@ -110,6 +110,7 @@ import org.apache.calcite.sql.validate.SqlConformance; import org.apache.calcite.util.Glossary; import org.apache.calcite.util.NlsString; import org.apache.calcite.util.Pair; +import org.apache.calcite.util.SourceStringReader; import org.apache.calcite.util.Util; import org.apache.calcite.util.trace.CalciteTrace; @@ -153,8 +154,14 @@ public class ${parser.class} extends SqlAbstractParserImpl * {@link SqlParserImplFactory} implementation for creating parser. */ public static final SqlParserImplFactory FACTORY = new SqlParserImplFactory() { - public SqlAbstractParserImpl getParser(Reader stream) { - return new ${parser.class}(stream); + public SqlAbstractParserImpl getParser(Reader reader) { + final ${parser.class} parser = new ${parser.class}(reader); + if (reader instanceof SourceStringReader) { + final String sql = + ((SourceStringReader) reader).getSourceString(); + parser.setOriginalSql(sql); + } + return parser; } }; http://git-wip-us.apache.org/repos/asf/calcite/blob/03c88b69/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 382b2d7..5305e91 100644 --- a/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java +++ b/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java @@ -55,6 +55,7 @@ import org.apache.calcite.util.Util; import com.google.common.collect.ImmutableList; +import java.io.Reader; import java.util.List; import java.util.Properties; @@ -162,14 +163,14 @@ public class PlannerImpl implements Planner, ViewExpander { } } - public SqlNode parse(final String sql) throws SqlParseException { + public SqlNode parse(final Reader reader) throws SqlParseException { switch (state) { case STATE_0_CLOSED: case STATE_1_RESET: ready(); } ensure(State.STATE_2_READY); - SqlParser parser = SqlParser.create(sql, parserConfig); + SqlParser parser = SqlParser.create(reader, parserConfig); SqlNode sqlNode = parser.parseStmt(); state = State.STATE_3_PARSED; return sqlNode; http://git-wip-us.apache.org/repos/asf/calcite/blob/03c88b69/core/src/main/java/org/apache/calcite/sql/parser/SqlParser.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/parser/SqlParser.java b/core/src/main/java/org/apache/calcite/sql/parser/SqlParser.java index 0e3aa22..50772b4 100644 --- a/core/src/main/java/org/apache/calcite/sql/parser/SqlParser.java +++ b/core/src/main/java/org/apache/calcite/sql/parser/SqlParser.java @@ -25,7 +25,9 @@ import org.apache.calcite.sql.parser.impl.SqlParserImpl; import org.apache.calcite.sql.validate.SqlConformance; import org.apache.calcite.sql.validate.SqlConformanceEnum; import org.apache.calcite.sql.validate.SqlDelegatingConformance; +import org.apache.calcite.util.SourceStringReader; +import java.io.Reader; import java.io.StringReader; import java.util.Objects; @@ -40,14 +42,11 @@ public class SqlParser { //~ Instance fields -------------------------------------------------------- private final SqlAbstractParserImpl parser; - private final String originalInput; //~ Constructors ----------------------------------------------------------- - private SqlParser(String s, SqlAbstractParserImpl parser, + private SqlParser(SqlAbstractParserImpl parser, Config config) { - this.originalInput = s; this.parser = parser; - parser.setOriginalSql(s); parser.setTabSize(1); parser.setQuotedCasing(config.quotedCasing()); parser.setUnquotedCasing(config.unquotedCasing()); @@ -88,15 +87,33 @@ public class SqlParser { * parser implementation created from given {@link SqlParserImplFactory} * with given quoting syntax and casing policies for identifiers. * - * @param sql A SQL statement or expression to parse. + * @param sql A SQL statement or expression to parse * @param config The parser configuration (identifier max length, etc.) * @return A parser */ public static SqlParser create(String sql, Config config) { + return create(new SourceStringReader(sql), config); + } + + /** + * Creates a <code>SqlParser</code> to parse the given string using the + * parser implementation created from given {@link SqlParserImplFactory} + * with given quoting syntax and casing policies for identifiers. + * + * <p>Unlike + * {@link #create(java.lang.String, org.apache.calcite.sql.parser.SqlParser.Config)}, + * the parser is not able to return the original query string, but will + * instead return "?". + * + * @param reader The source for the SQL statement or expression to parse + * @param config The parser configuration (identifier max length, etc.) + * @return A parser + */ + public static SqlParser create(Reader reader, Config config) { SqlAbstractParserImpl parser = - config.parserFactory().getParser(new StringReader(sql)); + config.parserFactory().getParser(reader); - return new SqlParser(sql, parser, config); + return new SqlParser(parser, config); } /** @@ -108,10 +125,11 @@ public class SqlParser { try { return parser.parseSqlExpressionEof(); } catch (Throwable ex) { - if ((ex instanceof CalciteContextException) - && (originalInput != null)) { - ((CalciteContextException) ex).setOriginalStatement( - originalInput); + if (ex instanceof CalciteContextException) { + final String originalSql = parser.getOriginalSql(); + if (originalSql != null) { + ((CalciteContextException) ex).setOriginalStatement(originalSql); + } } throw parser.normalizeException(ex); } @@ -129,10 +147,11 @@ public class SqlParser { try { return parser.parseSqlStmtEof(); } catch (Throwable ex) { - if ((ex instanceof CalciteContextException) - && (originalInput != null)) { - ((CalciteContextException) ex).setOriginalStatement( - originalInput); + if (ex instanceof CalciteContextException) { + final String originalSql = parser.getOriginalSql(); + if (originalSql != null) { + ((CalciteContextException) ex).setOriginalStatement(originalSql); + } } throw parser.normalizeException(ex); } http://git-wip-us.apache.org/repos/asf/calcite/blob/03c88b69/core/src/main/java/org/apache/calcite/tools/Planner.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/tools/Planner.java b/core/src/main/java/org/apache/calcite/tools/Planner.java index 2d9cd82..e2b84c9 100644 --- a/core/src/main/java/org/apache/calcite/tools/Planner.java +++ b/core/src/main/java/org/apache/calcite/tools/Planner.java @@ -24,6 +24,9 @@ import org.apache.calcite.rel.type.RelDataTypeFactory; import org.apache.calcite.sql.SqlNode; import org.apache.calcite.sql.parser.SqlParseException; import org.apache.calcite.util.Pair; +import org.apache.calcite.util.SourceStringReader; + +import java.io.Reader; /** * A façade that covers Calcite's query planning process: parse SQL, @@ -43,7 +46,19 @@ public interface Planner extends AutoCloseable { * @return The root node of the SQL parse tree. * @throws org.apache.calcite.sql.parser.SqlParseException on parse error */ - SqlNode parse(String sql) throws SqlParseException; + default SqlNode parse(String sql) throws SqlParseException { + return parse(new SourceStringReader(sql)); + } + + /** + * Parses and validates a SQL statement. + * + * @param source A reader which will provide the SQL statement to parse. + * + * @return The root node of the SQL parse tree. + * @throws org.apache.calcite.sql.parser.SqlParseException on parse error + */ + SqlNode parse(Reader source) throws SqlParseException; /** * Validates a SQL statement. http://git-wip-us.apache.org/repos/asf/calcite/blob/03c88b69/core/src/main/java/org/apache/calcite/util/SourceStringReader.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/util/SourceStringReader.java b/core/src/main/java/org/apache/calcite/util/SourceStringReader.java new file mode 100644 index 0000000..38c2c69 --- /dev/null +++ b/core/src/main/java/org/apache/calcite/util/SourceStringReader.java @@ -0,0 +1,46 @@ +/* + * 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.util; + +import java.io.StringReader; +import java.util.Objects; +import javax.annotation.Nonnull; + +/** + * Extension to {@link StringReader} that allows the original string to be + * recovered. + */ +public class SourceStringReader extends StringReader { + private final String s; + + /** + * Creates a source string reader. + * + * @param s String providing the character stream + */ + public SourceStringReader(@Nonnull String s) { + super(Objects.requireNonNull(s)); + this.s = s; + } + + /** Returns the source string. */ + public @Nonnull String getSourceString() { + return s; + } +} + +// End SourceStringReader.java http://git-wip-us.apache.org/repos/asf/calcite/blob/03c88b69/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 af49b2f..421cca5 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 @@ -30,6 +30,7 @@ import org.apache.calcite.test.DiffTestCase; import org.apache.calcite.test.SqlValidatorTestCase; import org.apache.calcite.util.Bug; import org.apache.calcite.util.ConversionUtil; +import org.apache.calcite.util.SourceStringReader; import org.apache.calcite.util.Sources; import org.apache.calcite.util.TestUtil; import org.apache.calcite.util.Util; @@ -49,6 +50,8 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.PrintWriter; +import java.io.Reader; +import java.io.StringReader; import java.nio.file.Paths; import java.util.Arrays; import java.util.List; @@ -595,7 +598,11 @@ public class SqlParserTest { } protected SqlParser getSqlParser(String sql) { - return SqlParser.create(sql, + return getSqlParser(new SourceStringReader(sql)); + } + + protected SqlParser getSqlParser(Reader source) { + return SqlParser.create(source, SqlParser.configBuilder() .setParserFactory(parserImplFactory()) .setQuoting(quoting) @@ -8432,6 +8439,15 @@ public class SqlParserTest { "('100' IS NOT JSON SCALAR)"); } + @Test public void testParseWithReader() throws Exception { + String query = "select * from dual"; + SqlParser sqlParserReader = getSqlParser(new StringReader(query)); + SqlNode node1 = sqlParserReader.parseQuery(); + SqlParser sqlParserString = getSqlParser(query); + SqlNode node2 = sqlParserString.parseQuery(); + assertEquals(node2.toString(), node1.toString()); + } + //~ Inner Interfaces ------------------------------------------------------- /** http://git-wip-us.apache.org/repos/asf/calcite/blob/03c88b69/core/src/test/java/org/apache/calcite/sql/test/SqlTestFactory.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlTestFactory.java b/core/src/test/java/org/apache/calcite/sql/test/SqlTestFactory.java index a1f1ad1..428f286 100644 --- a/core/src/test/java/org/apache/calcite/sql/test/SqlTestFactory.java +++ b/core/src/test/java/org/apache/calcite/sql/test/SqlTestFactory.java @@ -36,6 +36,7 @@ import org.apache.calcite.test.CalciteAssert; import org.apache.calcite.test.MockSqlOperatorTable; import org.apache.calcite.test.catalog.MockCatalogReader; import org.apache.calcite.test.catalog.MockCatalogReaderSimple; +import org.apache.calcite.util.SourceStringReader; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; @@ -112,7 +113,7 @@ public class SqlTestFactory { } public SqlParser createParser(String sql) { - return SqlParser.create(sql, parserConfig.get()); + return SqlParser.create(new SourceStringReader(sql), parserConfig.get()); } public static SqlParser.Config createParserConfig(ImmutableMap<String, Object> options) { http://git-wip-us.apache.org/repos/asf/calcite/blob/03c88b69/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java index c59f3a1..9c55b1f 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -22,11 +22,14 @@ import org.apache.calcite.avatica.util.TimeUnit; import org.apache.calcite.config.Lex; import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rel.type.RelDataTypeSystem; +import org.apache.calcite.runtime.CalciteContextException; import org.apache.calcite.sql.SqlCollation; +import org.apache.calcite.sql.SqlNode; import org.apache.calcite.sql.SqlOperator; import org.apache.calcite.sql.SqlSpecialOperator; import org.apache.calcite.sql.fun.OracleSqlOperatorTable; import org.apache.calcite.sql.fun.SqlStdOperatorTable; +import org.apache.calcite.sql.parser.SqlParser; import org.apache.calcite.sql.test.SqlTester; import org.apache.calcite.sql.type.ArraySqlType; import org.apache.calcite.sql.type.SqlTypeFactoryImpl; @@ -53,6 +56,7 @@ import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.StringReader; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.nio.charset.Charset; @@ -63,11 +67,16 @@ import java.util.Locale; import java.util.Objects; import java.util.function.Consumer; +import static org.apache.calcite.sql.parser.SqlParser.configBuilder; + import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.nullValue; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * Concrete child class of {@link SqlValidatorTestCase}, containing lots of unit @@ -10780,6 +10789,44 @@ public class SqlValidatorTest extends SqlValidatorTestCase { checkExpType("'100' is not json scalar", "BOOLEAN NOT NULL"); checkExpFails("^100 is json value^", "(?s).*Cannot apply.*"); } + + @Test public void testValidatorReportsOriginalQueryUsingReader() + throws Exception { + final String sql = "select a from b"; + final SqlParser.Config config = configBuilder().build(); + final String messagePassingSqlString; + try { + final SqlParser sqlParserReader = SqlParser.create(sql, config); + final SqlNode node = sqlParserReader.parseQuery(); + final SqlValidator validator = tester.getValidator(); + final SqlNode x = validator.validate(node); + fail("expecting an error, got " + x); + return; + } catch (CalciteContextException error) { + // we want the exception to report column and line + assertThat(error.getMessage(), + is("At line 1, column 15: Object 'B' not found")); + messagePassingSqlString = error.getMessage(); + // the error does not contain the original query (even using a String) + assertNull(error.getOriginalStatement()); + } + + // parse SQL text using a java.io.Reader and not a java.lang.String + try { + final SqlParser sqlParserReader = + SqlParser.create(new StringReader(sql), config); + final SqlNode node = sqlParserReader.parseQuery(); + final SqlValidator validator = tester.getValidator(); + final SqlNode x = validator.validate(node); + fail("expecting an error, got " + x); + } catch (CalciteContextException error) { + // we want exactly the same error as with java.lang.String input + assertThat(error.getMessage(), is(messagePassingSqlString)); + // the error does not contain the original query (using a Reader) + assertThat(error.getOriginalStatement(), nullValue()); + } + } + } // End SqlValidatorTest.java
