[CALCITE-2433] SqlAdvisor: support configurable quoting characters
Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/89e22e3f Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/89e22e3f Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/89e22e3f Branch: refs/heads/master Commit: 89e22e3ff8082790a3160d0a95cf3546e65a77d1 Parents: 76a8cfd Author: Vladimir Sitnikov <[email protected]> Authored: Wed Aug 1 13:55:44 2018 +0300 Committer: Vladimir Sitnikov <[email protected]> Committed: Wed Sep 5 15:24:24 2018 +0300 ---------------------------------------------------------------------- .../apache/calcite/sql/advise/SqlAdvisor.java | 35 +++++++-- .../calcite/sql/advise/SqlSimpleParser.java | 83 +++++++++++++------- .../apache/calcite/sql/test/SqlAdvisorTest.java | 82 +++++++++++++++++-- .../apache/calcite/sql/test/SqlTestFactory.java | 6 +- .../calcite/test/SqlValidatorTestCase.java | 30 +++++++ .../java/org/apache/calcite/test/WithLex.java | 35 +++++++++ 6 files changed, 226 insertions(+), 45 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/89e22e3f/core/src/main/java/org/apache/calcite/sql/advise/SqlAdvisor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/advise/SqlAdvisor.java b/core/src/main/java/org/apache/calcite/sql/advise/SqlAdvisor.java index cafa612..69f3d59 100644 --- a/core/src/main/java/org/apache/calcite/sql/advise/SqlAdvisor.java +++ b/core/src/main/java/org/apache/calcite/sql/advise/SqlAdvisor.java @@ -62,6 +62,7 @@ public class SqlAdvisor { // Flags indicating precision/scale combinations private final SqlValidatorWithHints validator; + private final SqlParser.Config parserConfig; //~ Constructors ----------------------------------------------------------- @@ -72,11 +73,33 @@ public class SqlAdvisor { */ public SqlAdvisor( SqlValidatorWithHints validator) { + this(validator, SqlParser.Config.DEFAULT); + } + + /** + * Creates a SqlAdvisor with a validator instance and given parser configuration + * + * @param validator Validator + * @param parserConfig parser config + */ + public SqlAdvisor( + SqlValidatorWithHints validator, + SqlParser.Config parserConfig) { this.validator = validator; + this.parserConfig = parserConfig; } //~ Methods ---------------------------------------------------------------- + private char quoteStart() { + return parserConfig.quoting().string.charAt(0); + } + + private char quoteEnd() { + char quote = quoteStart(); + return quote == '[' ? ']' : quote; + } + /** * Gets completion hints for a partially completed or syntactically incorrect * sql statement with cursor pointing to the position where completion hints @@ -107,7 +130,7 @@ public class SqlAdvisor { --wordStart; } if ((wordStart > 0) - && (sql.charAt(wordStart - 1) == '"')) { + && (sql.charAt(wordStart - 1) == quoteStart())) { quoted = true; --wordStart; } @@ -125,7 +148,7 @@ public class SqlAdvisor { } if (quoted && (wordEnd < sql.length()) - && (sql.charAt(wordEnd) == '"')) { + && (sql.charAt(wordEnd) == quoteEnd())) { ++wordEnd; } @@ -323,7 +346,7 @@ public class SqlAdvisor { * @return whether SQL statement is valid */ public boolean isValid(String sql) { - SqlSimpleParser simpleParser = new SqlSimpleParser(HINT_TOKEN); + SqlSimpleParser simpleParser = new SqlSimpleParser(HINT_TOKEN, parserConfig); String simpleSql = simpleParser.simplifySql(sql); SqlNode sqlNode; try { @@ -390,7 +413,7 @@ public class SqlAdvisor { * @return a completed, valid (and possibly simplified SQL statement */ public String simplifySql(String sql, int cursor) { - SqlSimpleParser parser = new SqlSimpleParser(HINT_TOKEN); + SqlSimpleParser parser = new SqlSimpleParser(HINT_TOKEN, parserConfig); return parser.simplifySql(sql, cursor); } @@ -419,7 +442,7 @@ public class SqlAdvisor { * @return metadata */ protected SqlAbstractParserImpl.Metadata getParserMetadata() { - SqlParser parser = SqlParser.create(""); + SqlParser parser = SqlParser.create("", parserConfig); return parser.getMetadata(); } @@ -433,7 +456,7 @@ public class SqlAdvisor { * @throws SqlParseException if not syntactically valid */ protected SqlNode parseQuery(String sql) throws SqlParseException { - SqlParser parser = SqlParser.create(sql); + SqlParser parser = SqlParser.create(sql, parserConfig); return parser.parseStmt(); } http://git-wip-us.apache.org/repos/asf/calcite/blob/89e22e3f/core/src/main/java/org/apache/calcite/sql/advise/SqlSimpleParser.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/advise/SqlSimpleParser.java b/core/src/main/java/org/apache/calcite/sql/advise/SqlSimpleParser.java index f9a0d31..9765cd9 100644 --- a/core/src/main/java/org/apache/calcite/sql/advise/SqlSimpleParser.java +++ b/core/src/main/java/org/apache/calcite/sql/advise/SqlSimpleParser.java @@ -16,6 +16,9 @@ */ package org.apache.calcite.sql.advise; +import org.apache.calcite.avatica.util.Quoting; +import org.apache.calcite.sql.parser.SqlParser; + import java.util.ArrayList; import java.util.EnumSet; import java.util.HashMap; @@ -88,6 +91,7 @@ public class SqlSimpleParser { //~ Instance fields -------------------------------------------------------- private final String hintToken; + private final SqlParser.Config parserConfig; //~ Constructors ----------------------------------------------------------- @@ -95,9 +99,23 @@ public class SqlSimpleParser { * Creates a SqlSimpleParser * * @param hintToken Hint token + * @deprecated */ + @Deprecated public SqlSimpleParser(String hintToken) { + this(hintToken, SqlParser.Config.DEFAULT); + } + + /** + * Creates a SqlSimpleParser + * + * @param hintToken Hint token + * @param parserConfig parser configuration + */ + public SqlSimpleParser(String hintToken, + SqlParser.Config parserConfig) { this.hintToken = hintToken; + this.parserConfig = parserConfig; } //~ Methods ---------------------------------------------------------------- @@ -131,7 +149,7 @@ public class SqlSimpleParser { * @return a completed, valid (and possibly simplified) SQL statement */ public String simplifySql(String sql) { - Tokenizer tokenizer = new Tokenizer(sql, hintToken); + Tokenizer tokenizer = new Tokenizer(sql, hintToken, parserConfig.quoting()); List<Token> list = new ArrayList<Token>(); while (true) { Token token = tokenizer.nextToken(); @@ -249,15 +267,46 @@ public class SqlSimpleParser { final String sql; private final String hintToken; + private final char openQuote; private int pos; int start = 0; + @Deprecated public Tokenizer(String sql, String hintToken) { + this(sql, hintToken, Quoting.DOUBLE_QUOTE); + } + + public Tokenizer(String sql, String hintToken, Quoting quoting) { this.sql = sql; this.hintToken = hintToken; + this.openQuote = quoting.string.charAt(0); this.pos = 0; } + private Token parseQuotedIdentifier() { + // Parse double-quoted identifier. + start = pos; + ++pos; + char closeQuote = openQuote == '[' ? ']' : openQuote; + while (pos < sql.length()) { + char c = sql.charAt(pos); + ++pos; + if (c == closeQuote) { + if (pos < sql.length() && sql.charAt(pos) == closeQuote) { + // Double close means escaped closing quote is a part of identifer + ++pos; + continue; + } + break; + } + } + String match = sql.substring(start, pos); + if (match.startsWith(openQuote + " " + hintToken + " ")) { + return new Token(TokenType.ID, hintToken); + } + return new Token(TokenType.DQID, match); + } + public Token nextToken() { while (pos < sql.length()) { char c = sql.charAt(pos); @@ -275,35 +324,6 @@ public class SqlSimpleParser { ++pos; return new Token(TokenType.RPAREN); - case '"': - - // Parse double-quoted identifier. - start = pos; - ++pos; - while (pos < sql.length()) { - c = sql.charAt(pos); - ++pos; - if (c == '"') { - if (pos < sql.length()) { - char c1 = sql.charAt(pos); - if (c1 == '"') { - // encountered consecutive - // double-quotes; still in identifier - ++pos; - } else { - break; - } - } else { - break; - } - } - } - match = sql.substring(start, pos); - if (match.startsWith("\" " + hintToken + " ")) { - return new Token(TokenType.ID, hintToken); - } - return new Token(TokenType.DQID, match); - case '\'': // Parse single-quoted identifier. @@ -353,6 +373,9 @@ public class SqlSimpleParser { } default: + if (c == openQuote) { + return parseQuotedIdentifier(); + } if (Character.isWhitespace(c)) { ++pos; break; http://git-wip-us.apache.org/repos/asf/calcite/blob/89e22e3f/core/src/test/java/org/apache/calcite/sql/test/SqlAdvisorTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlAdvisorTest.java b/core/src/test/java/org/apache/calcite/sql/test/SqlAdvisorTest.java index 634c5ff..fdd0881 100644 --- a/core/src/test/java/org/apache/calcite/sql/test/SqlAdvisorTest.java +++ b/core/src/test/java/org/apache/calcite/sql/test/SqlAdvisorTest.java @@ -16,16 +16,21 @@ */ package org.apache.calcite.sql.test; +import org.apache.calcite.config.Lex; import org.apache.calcite.sql.advise.SqlAdvisor; import org.apache.calcite.sql.advise.SqlAdvisorValidator; import org.apache.calcite.sql.advise.SqlSimpleParser; +import org.apache.calcite.sql.parser.SqlParser; import org.apache.calcite.sql.parser.SqlParserUtil; import org.apache.calcite.sql.validate.SqlMoniker; import org.apache.calcite.sql.validate.SqlMonikerType; import org.apache.calcite.test.SqlValidatorTestCase; +import org.apache.calcite.test.WithLex; import org.junit.Assert; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.MethodRule; import java.util.ArrayList; import java.util.Arrays; @@ -47,6 +52,8 @@ public class SqlAdvisorTest extends SqlValidatorTestCase { public static final SqlTestFactory ADVISOR_TEST_FACTORY = SqlTestFactory.INSTANCE.withValidator( SqlAdvisorValidator::new); + @Rule public MethodRule configureTester = SqlValidatorTestCase.TESTER_CONFIGURATION_RULE; + //~ Static fields/initializers --------------------------------------------- private static final List<String> STAR_KEYWORD = @@ -378,7 +385,8 @@ public class SqlAdvisorTest extends SqlValidatorTestCase { private void assertTokenizesTo(String sql, String expected) { SqlSimpleParser.Tokenizer tokenizer = - new SqlSimpleParser.Tokenizer(sql, "xxxxx"); + new SqlSimpleParser.Tokenizer(sql, "xxxxx", + tester.getFactory().getParserConfig().quoting()); StringBuilder buf = new StringBuilder(); while (true) { SqlSimpleParser.Token token = tokenizer.nextToken(); @@ -1115,23 +1123,51 @@ public class SqlAdvisorTest extends SqlValidatorTestCase { assertSimplify(sql, expected); } - @Test public void testSimpleParserQuotedId() { + @WithLex(Lex.SQL_SERVER) @Test public void testSimpleParserQuotedIdSqlServer() { + testSimpleParserQuotedIdImpl(); + } + + @WithLex(Lex.MYSQL) @Test public void testSimpleParserQuotedIdMySql() { + testSimpleParserQuotedIdImpl(); + } + + @WithLex(Lex.JAVA) @Test public void testSimpleParserQuotedIdJava() { + testSimpleParserQuotedIdImpl(); + } + + @Test public void testSimpleParserQuotedIdDefault() { + testSimpleParserQuotedIdImpl(); + } + + private String replaceQuotes(SqlParser.Config parserConfig, String sql) { + char openQuote = parserConfig.quoting().string.charAt(0); + char closeQuote = openQuote == '[' ? ']' : openQuote; + return sql.replace('[', openQuote).replace(']', closeQuote); + } + + private void testSimpleParserQuotedIdImpl() { + SqlParser.Config parserConfig = tester.getFactory().getParserConfig(); String sql; String expected; // unclosed double-quote - sql = "select * from t where \"^"; - expected = "SELECT * FROM t WHERE _suggest_"; + sql = replaceQuotes(parserConfig, "select * from t where [^"); + expected = replaceQuotes(parserConfig, "SELECT * FROM t WHERE _suggest_"); assertSimplify(sql, expected); // closed double-quote - sql = "select * from t where \"^\" and x = y"; - expected = "SELECT * FROM t WHERE _suggest_ and x = y"; + sql = replaceQuotes(parserConfig, "select * from t where [^] and x = y"); + expected = replaceQuotes(parserConfig, "SELECT * FROM t WHERE _suggest_ and x = y"); assertSimplify(sql, expected); // closed double-quote containing extra stuff - sql = "select * from t where \"^foo\" and x = y"; - expected = "SELECT * FROM t WHERE _suggest_ and x = y"; + sql = replaceQuotes(parserConfig, "select * from t where [^foo] and x = y"); + expected = replaceQuotes(parserConfig, "SELECT * FROM t WHERE _suggest_ and x = y"); + assertSimplify(sql, expected); + + // escaped double-quote containing extra stuff + sql = replaceQuotes(parserConfig, "select * from t where [^f]]oo] and x = y"); + expected = replaceQuotes(parserConfig, "SELECT * FROM t WHERE _suggest_ and x = y"); assertSimplify(sql, expected); } @@ -1204,6 +1240,27 @@ public class SqlAdvisorTest extends SqlValidatorTestCase { assertComplete(sql, "", null); } + @Test public void testNestSchema() throws Exception { + String sql; + sql = "select * from sales.n^"; + assertComplete( + sql, + Collections.singletonList("SCHEMA(CATALOG.SALES.NEST)")); + + sql = "select * from sales.n^asfasdf"; + assertComplete( + sql, + Collections.singletonList("SCHEMA(CATALOG.SALES.NEST)")); + + sql = "select * from sales.n^est"; + assertComplete( + sql, + Collections.singletonList("SCHEMA(CATALOG.SALES.NEST)")); + + sql = "select * from sales.nu^"; + assertComplete(sql, "", null); + } + @Test public void testUnion() throws Exception { // we simplify set ops such as UNION by removing other queries - // thereby avoiding validation errors due to mismatched select lists @@ -1224,6 +1281,15 @@ public class SqlAdvisorTest extends SqlValidatorTestCase { simplified = "SELECT * FROM emp GROUP BY _suggest_"; assertSimplify(sql, simplified); } + + @WithLex(Lex.SQL_SERVER) @Test public void testMssql() { + String sql = + "select 1 from [emp] union select 2 from [DEPT] a where ^ and deptno < 5"; + String simplified = + "SELECT * FROM [DEPT] a WHERE _suggest_ and deptno < 5"; + assertSimplify(sql, simplified); + assertComplete(sql, EXPR_KEYWORDS, Arrays.asList("TABLE(a)"), DEPT_COLUMNS); + } } // End SqlAdvisorTest.java http://git-wip-us.apache.org/repos/asf/calcite/blob/89e22e3f/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 4abf867..c3bc4a6 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 @@ -107,6 +107,10 @@ public class SqlTestFactory { return opTab; } + public SqlParser.Config getParserConfig() { + return parserConfig.get(); + } + public SqlParser createParser(String sql) { return SqlParser.create(sql, parserConfig.get()); } @@ -130,7 +134,7 @@ public class SqlTestFactory { public SqlAdvisor createAdvisor() { SqlValidator validator = getValidator(); if (validator instanceof SqlValidatorWithHints) { - return new SqlAdvisor((SqlValidatorWithHints) validator); + return new SqlAdvisor((SqlValidatorWithHints) validator, parserConfig.get()); } throw new UnsupportedOperationException( "Validator should implement SqlValidatorWithHints, actual validator is " + validator); http://git-wip-us.apache.org/repos/asf/calcite/blob/89e22e3f/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java index a281c1c..9160d57 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java @@ -33,6 +33,10 @@ import org.apache.calcite.test.catalog.MockCatalogReaderExtended; import org.apache.calcite.util.TestUtil; import org.apache.calcite.util.Util; +import org.junit.rules.MethodRule; +import org.junit.runners.model.FrameworkMethod; +import org.junit.runners.model.Statement; + import java.nio.charset.Charset; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -76,6 +80,8 @@ public class SqlValidatorTestCase { new SqlTesterImpl(EXTENDED_TEST_FACTORY) .withConformance(SqlConformanceEnum.LENIENT); + public static final MethodRule TESTER_CONFIGURATION_RULE = new TesterConfigurationRule(); + //~ Instance fields -------------------------------------------------------- protected SqlTester tester; @@ -635,6 +641,30 @@ public class SqlValidatorTestCase { return new Sql(tester, sql.replace("^", ""), true); } } + + /** + * Enables to configure {@link #tester} behavior on a per-test basis. + * {@code tester} object is created in the test object constructor, and there's no + * trivial way to override its features. + * <p>This JUnit rule enables post-process test object on a per test method basis</p> + */ + private static class TesterConfigurationRule implements MethodRule { + @Override public Statement apply(Statement statement, FrameworkMethod frameworkMethod, + Object o) { + return new Statement() { + @Override public void evaluate() throws Throwable { + SqlValidatorTestCase tc = (SqlValidatorTestCase) o; + SqlTester tester = tc.tester; + WithLex lex = frameworkMethod.getAnnotation(WithLex.class); + if (lex != null) { + tester = tester.withLex(lex.value()); + } + tc.tester = tester; + statement.evaluate(); + } + }; + } + } } // End SqlValidatorTestCase.java http://git-wip-us.apache.org/repos/asf/calcite/blob/89e22e3f/core/src/test/java/org/apache/calcite/test/WithLex.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/WithLex.java b/core/src/test/java/org/apache/calcite/test/WithLex.java new file mode 100644 index 0000000..f6ef643 --- /dev/null +++ b/core/src/test/java/org/apache/calcite/test/WithLex.java @@ -0,0 +1,35 @@ +/* + * 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.test; + +import org.apache.calcite.config.Lex; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Annotation that indicates that test method should be run with given lex configuration. + */ +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.METHOD}) +public @interface WithLex { + Lex value() default Lex.ORACLE; +} + +// End WithLex.java
