From b975086d1b89a885a128bb87d6fa4eaa7545af50 Mon Sep 17 00:00:00 2001 From: Almog Gavra Date: Thu, 13 Jan 2022 13:16:26 -0800 Subject: [PATCH] fix: add GRACE and PERIOD to the nonReserved and automate testing (#8596) --- .../io/confluent/ksql/parser/SqlBase.g4 | 1 + .../ksql/parser/DefaultKsqlParser.java | 4 +- .../ksql/parser/ReservedWordMetaTest.java | 125 ++++++++++++++++++ 3 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 ksqldb-parser/src/test/java/io/confluent/ksql/parser/ReservedWordMetaTest.java diff --git a/ksqldb-parser/src/main/antlr4/io/confluent/ksql/parser/SqlBase.g4 b/ksqldb-parser/src/main/antlr4/io/confluent/ksql/parser/SqlBase.g4 index 7ffb80d5441e..dfdc14c478f9 100644 --- a/ksqldb-parser/src/main/antlr4/io/confluent/ksql/parser/SqlBase.g4 +++ b/ksqldb-parser/src/main/antlr4/io/confluent/ksql/parser/SqlBase.g4 @@ -336,6 +336,7 @@ nonReserved | EMIT | CHANGES | ESCAPE + | GRACE | PERIOD ; EMIT: 'EMIT'; diff --git a/ksqldb-parser/src/main/java/io/confluent/ksql/parser/DefaultKsqlParser.java b/ksqldb-parser/src/main/java/io/confluent/ksql/parser/DefaultKsqlParser.java index 316ee1171df5..1859be6fd052 100644 --- a/ksqldb-parser/src/main/java/io/confluent/ksql/parser/DefaultKsqlParser.java +++ b/ksqldb-parser/src/main/java/io/confluent/ksql/parser/DefaultKsqlParser.java @@ -15,6 +15,7 @@ package io.confluent.ksql.parser; +import com.google.common.annotations.VisibleForTesting; import io.confluent.ksql.metastore.TypeRegistry; import io.confluent.ksql.parser.SqlBaseParser.SingleStatementContext; import io.confluent.ksql.parser.exception.ParseFailedException; @@ -37,7 +38,8 @@ public class DefaultKsqlParser implements KsqlParser { // CHECKSTYLE_RULES.ON: ClassDataAbstractionCoupling - private static final BaseErrorListener ERROR_LISTENER = new BaseErrorListener() { + @VisibleForTesting + static final BaseErrorListener ERROR_LISTENER = new BaseErrorListener() { @Override public void syntaxError( final Recognizer recognizer, diff --git a/ksqldb-parser/src/test/java/io/confluent/ksql/parser/ReservedWordMetaTest.java b/ksqldb-parser/src/test/java/io/confluent/ksql/parser/ReservedWordMetaTest.java new file mode 100644 index 000000000000..09def2d896e6 --- /dev/null +++ b/ksqldb-parser/src/test/java/io/confluent/ksql/parser/ReservedWordMetaTest.java @@ -0,0 +1,125 @@ +/* + * Copyright 2020 Confluent Inc. + * + * Licensed under the Confluent Community License (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.confluent.io/confluent-community-license + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package io.confluent.ksql.parser; + +import com.google.common.collect.ImmutableSet; +import java.util.Set; +import java.util.stream.Collectors; +import org.antlr.v4.runtime.CharStreams; +import org.antlr.v4.runtime.CommonTokenStream; +import org.antlr.v4.runtime.Vocabulary; +import org.antlr.v4.runtime.atn.PredictionMode; +import org.apache.commons.lang3.text.WordUtils; +import org.hamcrest.Matchers; +import org.junit.Assert; +import org.junit.Ignore; +import org.junit.Test; +import static org.hamcrest.MatcherAssert.assertThat; + + +/** + * This test ensures that we don't accidentally add words to our grammar + * without adding them to our "nonReserved" list. This is necessary because + * adding words to our grammar is backwards incompatible if anyone has issued + * a query in the past that used on of those words. + */ +public class ReservedWordMetaTest { + + private static final String[] RESERVED = new String[]{"SELECT", "FROM", "AS", + "ALL", "DISTINCT", "WHERE", "WITHIN", "WINDOW", "GROUP", "BY", "HAVING", + "LIMIT", "AT", "OR", "AND", "IN", "NOT", "EXISTS", "BETWEEN", "LIKE", "IS", + "NULL", "TRUE", "FALSE", "MILLISECOND", "YEARS", "MONTHS", "DAYS", "HOURS", + "MINUTES", "SECONDS", "MILLISECONDS", "TUMBLING", "HOPPING", "SIZE", "ADVANCE", + "RETENTION", "CASE", "WHEN", "THEN", "ELSE", "END", "JOIN", "FULL", "OUTER", + "INNER", "LEFT", "RIGHT", "ON", "WITH", "VALUES", "CREATE", "TABLE", "TOPIC", + "STREAM", "STREAMS", "INSERT", "DELETE", "INTO", "DESCRIBE", "EXTENDED", + "PRINT", "CAST", "LIST", "TOPICS", "QUERY", "QUERIES", "TERMINATE", "LOAD", + "DROP", "TO", "RENAME", "SAMPLE", "EXPORT", "CATALOG", "PROPERTIES", + "BEGINNING", "UNSET", "RUN", "SCRIPT", "DECIMAL", "CONNECTOR", "CONNECTORS", + "NAMESPACE", "MATERIALIZED", "VIEW", "EQ", "NEQ", "LT", "LTE", "GT", "GTE", + "PLUS", "MINUS", "ASTERISK", "SLASH", "PERCENT", "CONCAT", "ASSIGN", + "STRUCT_FIELD_REF", "STRING", "INTEGER_VALUE", "DECIMAL_VALUE", + "FLOATING_POINT_VALUE", "IDENTIFIER", "DIGIT_IDENTIFIER", "QUOTED_IDENTIFIER", + "BACKQUOTED_IDENTIFIER", "TIME_WITH_TIME_ZONE", "TIMESTAMP_WITH_TIME_ZONE", + "SIMPLE_COMMENT", "BRACKETED_COMMENT", "WS", "UNRECOGNIZED"}; + private static final Set RESERVED_SET = ImmutableSet.copyOf(RESERVED); + + /** + * Enable this test to re-generate the grammar that is intentionally + * part of the reserved words. + */ + @Test + @Ignore + public void generateExcludedWords() { + final Vocabulary vocabulary = SqlBaseParser.VOCABULARY; + final int tokens = vocabulary.getMaxTokenType(); + final ImmutableSet.Builder builder = ImmutableSet.builder(); + + for (int i = 0; i < tokens; i++) { + final String symbolicName = vocabulary.getSymbolicName(i); + if (symbolicName != null) { + try { + buildParser(symbolicName).nonReserved(); + } catch (final ParsingException e) { + // word is reserved (not in nonReserved) + builder.add(symbolicName); + } + } + } + + final ImmutableSet reserved = builder.build(); + System.out.println(WordUtils.wrap("private static final String[] RESERVED = new String[]{" + + reserved + .stream() + .map(s -> '"' + s + '"') + .collect(Collectors.joining(", ")) + "};", 80, "\n\t\t", false)); + } + + @Test + public void shouldAddWordToNonReserved() { + final Vocabulary vocabulary = SqlBaseParser.VOCABULARY; + final int tokens = vocabulary.getMaxTokenType(); + + for (int i = 0; i < tokens; i++) { + final String symbolicName = vocabulary.getSymbolicName(i); + if (symbolicName != null) { + try { + buildParser(symbolicName).nonReserved(); + } catch (final ParsingException e) { + assertThat( + String.format("Detected new word ('%s') in the vocabulary that is reserved. " + + "This is a breaking change!! If this is intentional, then add it to " + + "the RESERVED set in this test, otherwise please add it to the nonReserved " + + "parser rule.", symbolicName), symbolicName, Matchers.isIn(RESERVED_SET)); + } + } + } + } + + private static SqlBaseParser buildParser(final String symbolicName) { + final SqlBaseLexer lexer = new SqlBaseLexer( + new CaseInsensitiveStream(CharStreams.fromString(symbolicName))); + + final CommonTokenStream tokenStream = new CommonTokenStream(lexer); + final SqlBaseParser parser = new SqlBaseParser(tokenStream); + parser.removeErrorListeners(); + parser.addErrorListener(DefaultKsqlParser.ERROR_LISTENER); + parser.getInterpreter().setPredictionMode(PredictionMode.LL); + + return parser; + } + +}