You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by bl...@apache.org on 2016/10/11 08:51:13 UTC
cassandra git commit: Exit query parsing upon first error
Repository: cassandra
Updated Branches:
refs/heads/cassandra-3.X e0a35e585 -> 830954304
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/cassandra-3.X
Commit: 830954304ebcf450b213c9ebc53cb4eefb87f02d
Parents: e0a35e5
Author: Berenguer Blasi <be...@gmail.com>
Authored: Tue Oct 11 10:47:36 2016 +0200
Committer: Benjamin Lerer <b....@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 + ' ' +