Repository: cassandra Updated Branches: refs/heads/trunk 9f1674926 -> dffe27077
Exit query parsing upon first error patch by Berenguer Blasi; reviewed by Benjamin Lerer for CASSANDRA-12598 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/83095430 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/83095430 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/83095430 Branch: refs/heads/trunk Commit: 830954304ebcf450b213c9ebc53cb4eefb87f02d Parents: e0a35e5 Author: Berenguer Blasi <berenguerbl...@gmail.com> Authored: Tue Oct 11 10:47:36 2016 +0200 Committer: Benjamin Lerer <b.le...@gmail.com> Committed: Tue Oct 11 10:50:01 2016 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + src/antlr/Cql.g | 17 +++++++ src/antlr/Parser.g | 48 +++++++++++++------- .../apache/cassandra/cql3/CqlParserTest.java | 15 +++--- .../validation/operations/AggregationTest.java | 4 +- 5 files changed, 61 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/83095430/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 00ff1eb..2f517e0 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.10 + * Exit query parsing upon first error (CASSANDRA-12598) * Fix cassandra-stress to use single seed in UUID generation (CASSANDRA-12729) * CQLSSTableWriter does not allow Update statement (CASSANDRA-12450) * Config class uses boxed types but DD exposes primitive types (CASSANDRA-12199) http://git-wip-us.apache.org/repos/asf/cassandra/blob/83095430/src/antlr/Cql.g ---------------------------------------------------------------------- diff --git a/src/antlr/Cql.g b/src/antlr/Cql.g index 61bdc43..a11f2fd 100644 --- a/src/antlr/Cql.g +++ b/src/antlr/Cql.g @@ -73,6 +73,23 @@ import Parser,Lexer; { gParser.addRecognitionError(msg); } + + //////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // Recovery methods are overridden to avoid wasting work on recovering from errors when the result will be + // ignored anyway. + //////////////////////////////////////////////////////////////////////////////////////////////////////////////// + + @Override + protected Object recoverFromMismatchedToken(IntStream input, int ttype, BitSet follow) throws RecognitionException + { + throw new MismatchedTokenException(ttype, input); + } + + @Override + public void recover(IntStream input, RecognitionException re) + { + // Do nothing. + } } @lexer::header { http://git-wip-us.apache.org/repos/asf/cassandra/blob/83095430/src/antlr/Parser.g ---------------------------------------------------------------------- diff --git a/src/antlr/Parser.g b/src/antlr/Parser.g index 4fa27a0..b3a9f5c 100644 --- a/src/antlr/Parser.g +++ b/src/antlr/Parser.g @@ -181,6 +181,22 @@ options { } } + //////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // Recovery methods are overridden to avoid wasting work on recovering from errors when the result will be + // ignored anyway. + //////////////////////////////////////////////////////////////////////////////////////////////////////////////// + + @Override + protected Object recoverFromMismatchedToken(IntStream input, int ttype, BitSet follow) throws RecognitionException + { + throw new MismatchedTokenException(ttype, input); + } + + @Override + public void recover(IntStream input, RecognitionException re) + { + // Do nothing. + } } /** STATEMENTS **/ @@ -411,12 +427,12 @@ updateStatement returns [UpdateStatement.ParsedUpdate expr] K_WHERE wclause=whereClause ( K_IF ( K_EXISTS { ifExists = true; } | conditions=updateConditions ))? { - return new UpdateStatement.ParsedUpdate(cf, - attrs, - operations, - wclause.build(), - conditions == null ? Collections.<Pair<ColumnDefinition.Raw, ColumnCondition.Raw>>emptyList() : conditions, - ifExists); + $expr = new UpdateStatement.ParsedUpdate(cf, + attrs, + operations, + wclause.build(), + conditions == null ? Collections.<Pair<ColumnDefinition.Raw, ColumnCondition.Raw>>emptyList() : conditions, + ifExists); } ; @@ -445,12 +461,12 @@ deleteStatement returns [DeleteStatement.Parsed expr] K_WHERE wclause=whereClause ( K_IF ( K_EXISTS { ifExists = true; } | conditions=updateConditions ))? { - return new DeleteStatement.Parsed(cf, - attrs, - columnDeletions, - wclause.build(), - conditions == null ? Collections.<Pair<ColumnDefinition.Raw, ColumnCondition.Raw>>emptyList() : conditions, - ifExists); + $expr = new DeleteStatement.Parsed(cf, + attrs, + columnDeletions, + wclause.build(), + conditions == null ? Collections.<Pair<ColumnDefinition.Raw, ColumnCondition.Raw>>emptyList() : conditions, + ifExists); } ; @@ -506,7 +522,7 @@ batchStatement returns [BatchStatement.Parsed expr] ( s=batchStatementObjective ';'? { statements.add(s); } )* K_APPLY K_BATCH { - return new BatchStatement.Parsed(type, attrs, statements); + $expr = new BatchStatement.Parsed(type, attrs, statements); } ; @@ -1197,12 +1213,12 @@ columnFamilyName returns [CFName name] ; userTypeName returns [UTName name] - : (ks=noncol_ident '.')? ut=non_type_ident { return new UTName(ks, ut); } + : (ks=noncol_ident '.')? ut=non_type_ident { $name = new UTName(ks, ut); } ; userOrRoleName returns [RoleName name] - @init { $name = new RoleName(); } - : roleName[name] {return $name;} + @init { RoleName role = new RoleName(); } + : roleName[role] {$name = role;} ; ksName[KeyspaceElementName name] http://git-wip-us.apache.org/repos/asf/cassandra/blob/83095430/test/unit/org/apache/cassandra/cql3/CqlParserTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/CqlParserTest.java b/test/unit/org/apache/cassandra/cql3/CqlParserTest.java index 13c4685..4b76dbc 100644 --- a/test/unit/org/apache/cassandra/cql3/CqlParserTest.java +++ b/test/unit/org/apache/cassandra/cql3/CqlParserTest.java @@ -36,7 +36,7 @@ public class CqlParserTest SyntaxErrorCounter firstCounter = new SyntaxErrorCounter(); SyntaxErrorCounter secondCounter = new SyntaxErrorCounter(); - CharStream stream = new ANTLRStringStream("SELECT * FORM users"); + CharStream stream = new ANTLRStringStream("SELECT * FORM FROM test"); CqlLexer lexer = new CqlLexer(stream); TokenStream tokenStream = new CommonTokenStream(lexer); @@ -44,11 +44,14 @@ public class CqlParserTest parser.addErrorListener(firstCounter); parser.addErrorListener(secondCounter); - parser.query(); + // By default CqlParser should recover from the syntax error by removing FORM + // but as recoverFromMismatchedToken and recover have been overloaded, it will not + // and the returned ParsedStatement will be null. + assertNull(parser.query()); - // ANTLR 3.5 reports 2 errors in the sentence above (missing FROM and missing EOF). - assertTrue(firstCounter.count > 0); - assertTrue(secondCounter.count > 0); + // Only one error must be reported (mismatched: FORM). + assertEquals(1, firstCounter.count); + assertEquals(1, secondCounter.count); } @Test @@ -68,7 +71,7 @@ public class CqlParserTest parser.query(); - assertTrue(firstCounter.count > 0); + assertEquals(1, firstCounter.count); assertEquals(0, secondCounter.count); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/83095430/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java index 49ce6f3..26f9210 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java @@ -1381,14 +1381,14 @@ public class AggregationTest extends CQLTester "FINALFUNC " + shortFunctionName(fFinal) + ' ' + "INITCOND 1"); - assertInvalidMessage("missing EOF", // specifying a function using "keyspace.functionname" is a syntax error + assertInvalidMessage("expecting EOF", // specifying a function using "keyspace.functionname" is a syntax error "CREATE AGGREGATE " + KEYSPACE_PER_TEST + ".test_wrong_ks(int) " + "SFUNC " + shortFunctionName(fState) + ' ' + "STYPE " + type + " " + "FINALFUNC " + fFinalWrong + ' ' + "INITCOND 1"); - assertInvalidMessage("missing EOF", // specifying a function using "keyspace.functionname" is a syntax error + assertInvalidMessage("expecting EOF", // specifying a function using "keyspace.functionname" is a syntax error "CREATE AGGREGATE " + KEYSPACE_PER_TEST + ".test_wrong_ks(int) " + "SFUNC " + shortFunctionName(fState) + ' ' + "STYPE " + type + ' ' +
