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 <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 + ' ' +

Reply via email to