diff --git a/ksqldb-engine/src/main/java/io/confluent/ksql/function/InternalFunctionRegistry.java b/ksqldb-engine/src/main/java/io/confluent/ksql/function/InternalFunctionRegistry.java index 597952adac84..2293631a50dd 100644 --- a/ksqldb-engine/src/main/java/io/confluent/ksql/function/InternalFunctionRegistry.java +++ b/ksqldb-engine/src/main/java/io/confluent/ksql/function/InternalFunctionRegistry.java @@ -24,6 +24,7 @@ import io.confluent.ksql.name.FunctionName; import io.confluent.ksql.schema.ksql.types.SqlType; import io.confluent.ksql.util.KsqlException; +import io.confluent.ksql.util.ParserKeywordValidatorUtil; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -38,7 +39,7 @@ public class InternalFunctionRegistry implements MutableFunctionRegistry { private final Map udfs = new HashMap<>(); private final Map udafs = new HashMap<>(); private final Map udtfs = new HashMap<>(); - private final FunctionNameValidator functionNameValidator = new FunctionNameValidator(); + private final ParserKeywordValidatorUtil functionNameValidator = new ParserKeywordValidatorUtil(); public InternalFunctionRegistry() { new BuiltInInitializer(this).init(); 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 8bac306cbe56..65ab58bdfaa2 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 @@ -19,8 +19,10 @@ import io.confluent.ksql.parser.SqlBaseParser.SingleStatementContext; import io.confluent.ksql.parser.exception.ParseFailedException; import io.confluent.ksql.parser.tree.Statement; +import io.confluent.ksql.util.ParserUtil; import java.util.List; import java.util.function.Function; +import java.util.regex.Matcher; import java.util.stream.Collectors; import org.antlr.v4.runtime.BaseErrorListener; import org.antlr.v4.runtime.CharStream; @@ -29,6 +31,7 @@ import org.antlr.v4.runtime.ParserRuleContext; import org.antlr.v4.runtime.RecognitionException; import org.antlr.v4.runtime.Recognizer; +import org.antlr.v4.runtime.Token; import org.antlr.v4.runtime.atn.PredictionMode; import org.antlr.v4.runtime.misc.Interval; import org.antlr.v4.runtime.misc.ParseCancellationException; @@ -47,7 +50,17 @@ public void syntaxError( final String message, final RecognitionException e ) { - throw new ParsingException(message, e, line, charPositionInLine); + if (offendingSymbol instanceof Token && isKeywordError( + message, ((Token) offendingSymbol).getText())) { + //Checks if the error is a reserved keyword error + final String tokenName = ((Token) offendingSymbol).getText(); + final String newMessage = + "\"" + tokenName + "\" is a reserved keyword and it can't be used as an identifier." + + " You can use it as an identifier by escaping it as \'" + tokenName + "\' "; + throw new ParsingException(newMessage, e, line, charPositionInLine); + } else { + throw new ParsingException(message, e, line, charPositionInLine); + } } }; @@ -130,4 +143,16 @@ private static String getStatementString(final SingleStatementContext singleStat singleStatementContext.stop.getStopIndex() )); } + + /** + * checks if the error is a reserved keyword error by checking the message and offendingSymbol + * @param message the error message + * @param offendingSymbol the symbol that caused the error + * @return + */ + private static boolean isKeywordError(final String message, final String offendingSymbol) { + final Matcher m = ParserUtil.EXTRANEOUS_INPUT_PATTERN.matcher(message); + + return m.find() && ParserUtil.isReserved(offendingSymbol); + } } diff --git a/ksqldb-engine/src/main/java/io/confluent/ksql/function/FunctionNameValidator.java b/ksqldb-parser/src/main/java/io/confluent/ksql/util/ParserKeywordValidatorUtil.java similarity index 86% rename from ksqldb-engine/src/main/java/io/confluent/ksql/function/FunctionNameValidator.java rename to ksqldb-parser/src/main/java/io/confluent/ksql/util/ParserKeywordValidatorUtil.java index 6ff234939d3f..f0caf82bccd5 100644 --- a/ksqldb-engine/src/main/java/io/confluent/ksql/function/FunctionNameValidator.java +++ b/ksqldb-parser/src/main/java/io/confluent/ksql/util/ParserKeywordValidatorUtil.java @@ -1,5 +1,5 @@ /* - * Copyright 2018 Confluent Inc. + * 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 @@ -13,7 +13,7 @@ * specific language governing permissions and limitations under the License. */ -package io.confluent.ksql.function; +package io.confluent.ksql.util; import com.google.common.collect.ImmutableSet; import io.confluent.ksql.parser.SqlBaseParser; @@ -24,11 +24,14 @@ import org.antlr.v4.runtime.Vocabulary; /** - * Check that a function name is valid. It is valid if it is not a Java reserved word - * and not a ksql reserved word and is a valid java identifier. + * This Class provides methods for checking whether a function name is valid. + * It is valid if it is not a Java reserved word and not a ksql reserved word + * and is a valid java identifier. + * It also provides utils for checking if keywords are reserved or not as + * defined in SqlBase.g4 */ @ThreadSafe -class FunctionNameValidator implements Predicate { +public class ParserKeywordValidatorUtil implements Predicate { private static final Set JAVA_RESERVED_WORDS = ImmutableSet.builder() .add("abstract").add("assert").add("boolean").add("break").add("byte").add("case") @@ -90,4 +93,8 @@ private static Set createFromVocabulary() { } return builder.build(); } + + public static Set getKsqlReservedWords() { + return KSQL_RESERVED_WORDS; + } } diff --git a/ksqldb-parser/src/main/java/io/confluent/ksql/util/ParserUtil.java b/ksqldb-parser/src/main/java/io/confluent/ksql/util/ParserUtil.java index d053c690afe1..2fd9f80841db 100644 --- a/ksqldb-parser/src/main/java/io/confluent/ksql/util/ParserUtil.java +++ b/ksqldb-parser/src/main/java/io/confluent/ksql/util/ParserUtil.java @@ -24,8 +24,10 @@ import io.confluent.ksql.execution.expression.tree.Literal; import io.confluent.ksql.execution.expression.tree.LongLiteral; import io.confluent.ksql.name.SourceName; +import io.confluent.ksql.parser.CaseInsensitiveStream; import io.confluent.ksql.parser.NodeLocation; import io.confluent.ksql.parser.ParsingException; +import io.confluent.ksql.parser.SqlBaseLexer; import io.confluent.ksql.parser.SqlBaseParser; import io.confluent.ksql.parser.SqlBaseParser.FloatLiteralContext; import io.confluent.ksql.parser.SqlBaseParser.IntegerLiteralContext; @@ -37,13 +39,17 @@ import java.util.HashMap; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.regex.Pattern; +import org.antlr.v4.runtime.CharStreams; +import org.antlr.v4.runtime.CommonTokenStream; import org.antlr.v4.runtime.ParserRuleContext; import org.antlr.v4.runtime.Token; import org.antlr.v4.runtime.tree.TerminalNode; +// CHECKSTYLE_RULES.OFF: ClassDataAbstractionCoupling public final class ParserUtil { - + // CHECKSTYLE_RULES.ON: ClassDataAbstractionCoupling /** * Source names must adhere to the kafka topic naming convention. We restrict * it here instead of as a parser rule to allow for a more descriptive error @@ -52,6 +58,8 @@ public final class ParserUtil { * @see org.apache.kafka.streams.state.StoreBuilder#name */ private static final Pattern VALID_SOURCE_NAMES = Pattern.compile("[a-zA-Z0-9_-]*"); + public static final Pattern EXTRANEOUS_INPUT_PATTERN = Pattern.compile( + "extraneous input.*expecting.*"); private ParserUtil() { } @@ -212,4 +220,32 @@ public static JsonObject convertJsonFieldCase(final JsonObject obj) { } return new JsonObject(convertedMap); } + + /** + * Checks if the token is a reserved keyword or not + * @param token the String that caused the parsing error + * @return true if the token is a reserved keyword according to SqlBase.g4 + * false otherwise + */ + public static boolean isReserved(final String token) { + + final SqlBaseLexer sqlBaseLexer = new SqlBaseLexer( + new CaseInsensitiveStream(CharStreams.fromString(token))); + final CommonTokenStream tokenStream = new CommonTokenStream(sqlBaseLexer); + final SqlBaseParser sqlBaseParser = new SqlBaseParser(tokenStream); + + sqlBaseParser.removeErrorListeners(); + + final SqlBaseParser.NonReservedContext nonReservedContext = sqlBaseParser.nonReserved(); + if (nonReservedContext.exception == null) { + // If we call nonReservedWord, and if it successfully parses, + // then we just parsed through a nonReserved word as defined in SqlBase.g4 + // and we return false + return false; + } + + final Set allVocab = ParserKeywordValidatorUtil.getKsqlReservedWords(); + + return allVocab.contains(token.toLowerCase()); + } } diff --git a/ksqldb-parser/src/test/java/io/confluent/ksql/parser/KsqlParserTest.java b/ksqldb-parser/src/test/java/io/confluent/ksql/parser/KsqlParserTest.java index 16eb6443ff99..a610631b2470 100644 --- a/ksqldb-parser/src/test/java/io/confluent/ksql/parser/KsqlParserTest.java +++ b/ksqldb-parser/src/test/java/io/confluent/ksql/parser/KsqlParserTest.java @@ -1128,6 +1128,50 @@ public void testSelectWithMissingComma() { assertThat(e.getMessage(), containsString("line 1:12: extraneous input 'C' expecting")); } + @Test + public void testReservedKeywordSyntaxError() { + // Given: + final String simpleQuery = "CREATE STREAM ORDERS (c1 VARCHAR, size INTEGER)\n" + + " WITH (kafka_topic='ords', value_format='json');"; + + // When: + final Exception e = assertThrows( + ParseFailedException.class, + () -> KsqlParserTestUtil.buildSingleAst(simpleQuery, metaStore) + ); + + // Then: + assertThat(e.getMessage(), containsString("\"size\" is a reserved keyword and it can't be used as an identifier")); + } + + @Test + public void testReservedKeywordSyntaxErrorCaseInsensitive() { + // Given: + final String simpleQuery = "CREATE STREAM ORDERS (Load VARCHAR, size INTEGER)\n" + + " WITH (kafka_topic='ords', value_format='json');"; + + // When: + final Exception e = assertThrows( + ParseFailedException.class, + () -> KsqlParserTestUtil.buildSingleAst(simpleQuery, metaStore) + ); + + // Then: + assertThat(e.getMessage(), containsString("\"Load\" is a reserved keyword and it can't be used as an identifier")); + } + + @Test + public void testNonReservedKeywordShouldNotThrowException() { + // 'sink' is a keyword but is non-reserved. should not throw an exception + final CreateStream result = (CreateStream) KsqlParserTestUtil.buildSingleAst("CREATE STREAM ORDERS" + + " (place VARCHAR, Sink INTEGER)\n" + + " WITH (kafka_topic='orders_topic', value_format='json');", metaStore).getStatement(); + + // Then: + assertThat(result.getName(), equalTo(SourceName.of("ORDERS"))); + assertThat(result.getProperties().getKafkaTopic(), equalTo("orders_topic")); + } + @Test public void testSelectWithMultipleFroms() { // Given: diff --git a/ksqldb-engine/src/test/java/io/confluent/ksql/function/FunctionNameValidatorTest.java b/ksqldb-parser/src/test/java/io/confluent/ksql/util/ParserKeywordValidatorUtilTest.java similarity index 92% rename from ksqldb-engine/src/test/java/io/confluent/ksql/function/FunctionNameValidatorTest.java rename to ksqldb-parser/src/test/java/io/confluent/ksql/util/ParserKeywordValidatorUtilTest.java index 08e38a5f2083..c11ccdcdb584 100644 --- a/ksqldb-engine/src/test/java/io/confluent/ksql/function/FunctionNameValidatorTest.java +++ b/ksqldb-parser/src/test/java/io/confluent/ksql/util/ParserKeywordValidatorUtilTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2018 Confluent Inc. + * 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 @@ -13,7 +13,7 @@ * specific language governing permissions and limitations under the License. */ -package io.confluent.ksql.function; +package io.confluent.ksql.util; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -22,9 +22,9 @@ import org.antlr.v4.runtime.Vocabulary; import org.junit.Test; -public class FunctionNameValidatorTest { +public class ParserKeywordValidatorUtilTest { - private final FunctionNameValidator validator = new FunctionNameValidator(); + private final ParserKeywordValidatorUtil validator = new ParserKeywordValidatorUtil(); @Test public void shouldNotAllowJavaReservedWords() { diff --git a/ksqldb-parser/src/test/java/io/confluent/ksql/util/ParserUtilTest.java b/ksqldb-parser/src/test/java/io/confluent/ksql/util/ParserUtilTest.java new file mode 100644 index 000000000000..b183a03941d1 --- /dev/null +++ b/ksqldb-parser/src/test/java/io/confluent/ksql/util/ParserUtilTest.java @@ -0,0 +1,43 @@ +package io.confluent.ksql.util; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public class ParserUtilTest { + + @Test + public void shouldBeReserved() { + // Given: + final String[] keywords = new String[]{ + "size", // reserved word + "load", // reserved word + "SIZE", //upper case + "Load" //case insensitive + }; + + // Then: + for (final String keyword : keywords) { + assertEquals(true, ParserUtil.isReserved(keyword)); + } + } + + @Test + public void shouldNotBeReserved() { + // Given: + final String[] keywords = new String[]{ + "source", // non-reserved keyword + "sink", // non-reserved keyword + "MAP", //upper case + "Array", //case insensitive + "ASSERT", + "foo", + "bAR" + }; + + // Then: + for (final String keyword : keywords) { + assertEquals(false, ParserUtil.isReserved(keyword)); + } + } +}