From dc457c5ebca5654c86b4df8a7574e579bae00b7e Mon Sep 17 00:00:00 2001
From: Aleksey Yeschenko <aleksey@apache.org>
Date: Fri, 26 Jul 2013 20:02:44 +0300
Subject: [PATCH] Allow empty IN relations in SELECT/UPDATE/DELETE statements

patch by Aleksey Yeschenko; reviewed by Sylvain Lebresne for
CASSANDRA-5626
---
 CHANGES.txt                                   |  1 +
 doc/cql3/CQL.textile                          | 12 ++--
 src/java/org/apache/cassandra/cql3/Cql.g      |  2 +-
 .../apache/cassandra/cql3/QueryProcessor.java |  2 +-
 .../statements/ModificationStatement.java     |  6 +-
 .../cql3/statements/SelectStatement.java      | 69 +++++++++----------
 .../cql3/statements/UpdateStatement.java      |  4 +-
 7 files changed, 51 insertions(+), 45 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index 077a9863cd..fe28eee9f6 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,6 +1,7 @@
 1.2.8
  * cqlsh: add collections support to COPY (CASSANDRA-5698)
  * retry important messages for any IOException (CASSANDRA-5804)
+ * Allow empty IN relations in SELECT/UPDATE/DELETE statements (CASSANDRA-5626)
 
 
 1.2.7
diff --git a/doc/cql3/CQL.textile b/doc/cql3/CQL.textile
index 44c4dd26c5..fe64c71f64 100644
--- a/doc/cql3/CQL.textile
+++ b/doc/cql3/CQL.textile
@@ -1,6 +1,6 @@
 <link rel="StyleSheet" href="CQL.css" type="text/css" media="screen">
 
-h1. Cassandra Query Language (CQL) v3.0.4
+h1. Cassandra Query Language (CQL) v3.0.5
 
 
  <span id="tableOfContents">
@@ -469,7 +469,7 @@ bc(syntax)..
                | <identifier> '[' <term> ']' '=' <term>
 
 <where-clause> ::= <identifier> '=' <term>
-                 | <identifier> IN '(' <term> ( ',' <term> )* ')'
+                 | <identifier> IN '(' ( <term> ( ',' <term> )* )? ')'
 
 <option> ::= TIMESTAMP <integer>
            | TTL <integer>
@@ -515,7 +515,7 @@ bc(syntax)..
 <selection> ::= <identifier> ( '[' <term> ']' )?
 
 <where-clause> ::= <identifier> '=' <term>
-                 | <identifier> IN '(' <term> ( ',' <term> )* ')'
+                 | <identifier> IN '(' ( <term> ( ',' <term> )* )? ')'
 p. 
 __Sample:__
 
@@ -597,7 +597,7 @@ bc(syntax)..
 <where-clause> ::= <relation> ( "AND" <relation> )*
 
 <relation> ::= <identifier> ("=" | "<" | ">" | "<=" | ">=") <term>
-             | <identifier> IN '(' <term> ( ',' <term>)* ')'
+             | <identifier> IN '(' ( <term> ( ',' <term>)* )? ')'
              | TOKEN '(' <identifier> ')' ("=" | "<" | ">" | "<=" | ">=") (<term> | TOKEN '( <term> ')' )
 
 <order-by> ::= <ordering> ( ',' <odering> )*
@@ -1052,6 +1052,10 @@ h2(#changes). Changes
 
 The following describes the addition/changes brought for each version of CQL.
 
+h3. 3.0.5
+
+* SELECT, UPDATE, and DELETE statements now allow empty IN relations (see "CASSANDRA-5626":https://issues.apache.org/jira/browse/CASSANDRA-5626).
+
 h3. 3.0.4
 
 * Updated the syntax for custom "secondary indexes":#createIndexStmt.
diff --git a/src/java/org/apache/cassandra/cql3/Cql.g b/src/java/org/apache/cassandra/cql3/Cql.g
index 3ee30d1bac..6b31da5152 100644
--- a/src/java/org/apache/cassandra/cql3/Cql.g
+++ b/src/java/org/apache/cassandra/cql3/Cql.g
@@ -799,7 +799,7 @@ relation[List<Relation> clauses]
                 $clauses.add(new Relation(id, type, t, true));
         }
     | name=cident K_IN { Relation rel = Relation.createInRelation($name.id); }
-       '(' f1=term { rel.addInValue(f1); } (',' fN=term { rel.addInValue(fN); } )* ')' { $clauses.add(rel); }
+       '(' ( f1=term { rel.addInValue(f1); } (',' fN=term { rel.addInValue(fN); } )* )? ')' { $clauses.add(rel); }
     ;
 
 comparatorType returns [CQL3Type t]
diff --git a/src/java/org/apache/cassandra/cql3/QueryProcessor.java b/src/java/org/apache/cassandra/cql3/QueryProcessor.java
index fa019c057a..4ae1e90190 100644
--- a/src/java/org/apache/cassandra/cql3/QueryProcessor.java
+++ b/src/java/org/apache/cassandra/cql3/QueryProcessor.java
@@ -41,7 +41,7 @@ import org.apache.cassandra.utils.SemanticVersion;
 
 public class QueryProcessor
 {
-    public static final SemanticVersion CQL_VERSION = new SemanticVersion("3.0.4");
+    public static final SemanticVersion CQL_VERSION = new SemanticVersion("3.0.5");
 
     private static final Logger logger = LoggerFactory.getLogger(QueryProcessor.class);
 
diff --git a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
index 322ce7fbcd..5ede98fd66 100644
--- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
@@ -85,11 +85,13 @@ public abstract class ModificationStatement extends CFStatement implements CQLSt
 
         validateConsistency(cl);
 
-        Collection<? extends IMutation> mutations = getMutations(variables, false, cl, queryState.getTimestamp());
-
         // The type should have been set by now or we have a bug
         assert type != null;
 
+        Collection<? extends IMutation> mutations = getMutations(variables, false, cl, queryState.getTimestamp());
+        if (mutations.isEmpty())
+            return null;
+
         switch (type)
         {
             case LOGGED:
diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
index 8343d1ee3f..4f559e5c28 100644
--- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
@@ -147,7 +147,7 @@ public class SelectStatement implements CQLStatement
     static List<Row> readLocally(String keyspace, List<ReadCommand> cmds)
     {
         Table table = Table.open(keyspace);
-        List<Row> rows = new ArrayList(cmds.size());
+        List<Row> rows = new ArrayList<Row>(cmds.size());
         for (ReadCommand cmd : cmds)
             rows.add(cmd.getRow(table));
         return rows;
@@ -201,7 +201,11 @@ public class SelectStatement implements CQLStatement
     private List<ReadCommand> getSliceCommands(List<ByteBuffer> variables) throws RequestValidationException
     {
         QueryPath queryPath = new QueryPath(columnFamily());
+
         Collection<ByteBuffer> keys = getKeys(variables);
+        if (keys.isEmpty()) // in case of IN () for (the last column of) the partition key.
+            return null;
+
         List<ReadCommand> commands = new ArrayList<ReadCommand>(keys.size());
 
         // ...a range (slice) of column names
@@ -227,13 +231,17 @@ public class SelectStatement implements CQLStatement
         else
         {
             // ByNames commands can share the filter
-            IDiskAtomFilter filter = makeFilter(variables); // names filter are never null
+            IDiskAtomFilter filter = makeFilter(variables);
+            if (filter == null)
+                return null;
+
             for (ByteBuffer key: keys)
             {
                 QueryProcessor.validateKey(key);
                 commands.add(new SliceByNamesReadCommand(keyspace(), key, queryPath, (NamesQueryFilter)filter));
             }
         }
+
         return commands;
     }
 
@@ -370,6 +378,8 @@ public class SelectStatement implements CQLStatement
         else
         {
             SortedSet<ByteBuffer> columnNames = getRequestedColumns(variables);
+            if (columnNames == null) // in case of IN () for the last column of the key
+                return null;
             QueryProcessor.validateColumnNames(columnNames);
             return new NamesQueryFilter(columnNames, true);
         }
@@ -404,7 +414,7 @@ public class SelectStatement implements CQLStatement
             }
             else
             {
-                if (r.eqValues.size() > 1)
+                if (r.isINRestriction())
                     throw new InvalidRequestException("IN is only supported on the last column of the partition key");
                 ByteBuffer val = r.eqValues.get(0).bindAndGet(variables);
                 if (val == null)
@@ -479,11 +489,13 @@ public class SelectStatement implements CQLStatement
         {
             ColumnIdentifier id = idIter.next();
             assert r != null && r.isEquality();
-            if (r.eqValues.size() > 1)
+            if (r.isINRestriction())
             {
                 // We have a IN, which we only support for the last column.
                 // If compact, just add all values and we're done. Otherwise,
                 // for each value of the IN, creates all the columns corresponding to the selection.
+                if (r.eqValues.isEmpty())
+                    return null;
                 SortedSet<ByteBuffer> columns = new TreeSet<ByteBuffer>(cfDef.cfm.comparator);
                 Iterator<Term> iter = r.eqValues.iterator();
                 while (iter.hasNext())
@@ -600,7 +612,7 @@ public class SelectStatement implements CQLStatement
 
             if (r.isEquality())
             {
-                if (r.eqValues.size() > 1)
+                if (r.isINRestriction())
                 {
                     // IN query, we only support it on the clustering column
                     assert name.position == names.size() - 1;
@@ -659,15 +671,13 @@ public class SelectStatement implements CQLStatement
             Restriction restriction = entry.getValue();
             if (restriction.isEquality())
             {
-                for (Term t : restriction.eqValues)
-                {
-                    ByteBuffer value = t.bindAndGet(variables);
-                    if (value == null)
-                        throw new InvalidRequestException(String.format("Unsupported null value for indexed column %s", name));
-                    if (value.remaining() > 0xFFFF)
-                        throw new InvalidRequestException("Index expression values may not be larger than 64K");
-                    expressions.add(new IndexExpression(name.name.key, IndexOperator.EQ, value));
-                }
+                assert restriction.eqValues.size() == 1; // IN is not supported for indexed columns.
+                ByteBuffer value = restriction.eqValues.get(0).bindAndGet(variables);
+                if (value == null)
+                    throw new InvalidRequestException(String.format("Unsupported null value for indexed column %s", name));
+                if (value.remaining() > 0xFFFF)
+                    throw new InvalidRequestException("Index expression values may not be larger than 64K");
+                expressions.add(new IndexExpression(name.name.key, IndexOperator.EQ, value));
             }
             else
             {
@@ -992,7 +1002,6 @@ public class SelectStatement implements CQLStatement
             CFDefinition cfDef = cfm.getCfDef();
 
             ColumnSpecification[] names = new ColumnSpecification[getBoundsTerms()];
-            IPartitioner partitioner = StorageService.getPartitioner();
 
             // Select clause
             if (parameters.isCount && !selectClause.isEmpty())
@@ -1008,7 +1017,7 @@ public class SelectStatement implements CQLStatement
              * WHERE clause. For a given entity, rules are:
              *   - EQ relation conflicts with anything else (including a 2nd EQ)
              *   - Can't have more than one LT(E) relation (resp. GT(E) relation)
-             *   - IN relation are restricted to row keys (for now) and conflics with anything else
+             *   - IN relation are restricted to row keys (for now) and conflicts with anything else
              *     (we could allow two IN for the same entity but that doesn't seem very useful)
              *   - The value_alias cannot be restricted in any way (we don't support wide rows with indexed value in CQL so far)
              */
@@ -1065,7 +1074,7 @@ public class SelectStatement implements CQLStatement
                 }
                 // We only support IN for the last name so far
                 // TODO: #3885 allows us to extend to other parts (cf. #4762)
-                else if (restriction.eqValues.size() > 1)
+                else if (restriction.isINRestriction())
                 {
                     if (i != stmt.columnRestrictions.length - 1)
                         throw new InvalidRequestException(String.format("PRIMARY KEY part %s cannot be restricted by IN relation", cname));
@@ -1105,12 +1114,9 @@ public class SelectStatement implements CQLStatement
                 }
                 else if (restriction.onToken)
                 {
-                    // If this is a query on tokens, it's necessary a range query (there can be more than one key per token), so reject IN queries (as we don't know how to do them)
+                    // If this is a query on tokens, it's necessarily a range query (there can be more than one key per token).
                     stmt.isKeyRange = true;
                     stmt.onToken = true;
-
-                    if (restriction.isEquality() && restriction.eqValues.size() > 1)
-                        throw new InvalidRequestException("Select using the token() function don't support IN clause");
                 }
                 else if (stmt.onToken)
                 {
@@ -1118,7 +1124,7 @@ public class SelectStatement implements CQLStatement
                 }
                 else if (restriction.isEquality())
                 {
-                    if (restriction.eqValues.size() > 1)
+                    if (restriction.isINRestriction())
                     {
                         // We only support IN for the last name so far
                         if (i != stmt.keyRestrictions.length - 1)
@@ -1137,7 +1143,6 @@ public class SelectStatement implements CQLStatement
             if (!stmt.metadataRestrictions.isEmpty())
             {
                 stmt.isKeyRange = true;
-                boolean hasEq = false;
                 Set<ByteBuffer> indexedNames = new HashSet<ByteBuffer>();
                 for (ColumnDefinition cfdef : cfm.getColumn_metadata().values())
                 {
@@ -1149,6 +1154,7 @@ public class SelectStatement implements CQLStatement
 
                 // Note: we cannot use idxManager.indexes() methods because we don't have a complete column name at this point, we only
                 // have the indexed component.
+                boolean hasEq = false;
                 for (Map.Entry<CFDefinition.Name, Restriction> entry : stmt.metadataRestrictions.entrySet())
                 {
                     Restriction restriction = entry.getValue();
@@ -1156,15 +1162,13 @@ public class SelectStatement implements CQLStatement
                         continue;
 
                     // We don't support IN for indexed values (basically this would require supporting a form of OR)
-                    if (restriction.eqValues.size() > 1)
+                    if (restriction.isINRestriction())
                         throw new InvalidRequestException("Cannot use IN operator on column not part of the partition key");
 
                     if (indexedNames.contains(entry.getKey().name.key))
-                    {
                         hasEq = true;
-                        break;
-                    }
                 }
+
                 if (!hasEq)
                     throw new InvalidRequestException("No indexed columns present in by-columns clause with Equal operator");
 
@@ -1270,7 +1274,7 @@ public class SelectStatement implements CQLStatement
                 receiver = new ColumnSpecification(name.ksName,
                                                    name.cfName,
                                                    new ColumnIdentifier("partition key token", true),
-                                                   StorageService.instance.getPartitioner().getTokenValidator());
+                                                   StorageService.getPartitioner().getTokenValidator());
             }
 
             switch (newRel.operator())
@@ -1370,14 +1374,9 @@ public class SelectStatement implements CQLStatement
             return eqValues != null;
         }
 
-        public void setBound(Bound b, Term t)
+        boolean isINRestriction()
         {
-            bounds[b.idx] = t;
-        }
-
-        public void setInclusive(Bound b)
-        {
-            boundInclusive[b.idx] = true;
+            return isEquality() && (eqValues.isEmpty() || eqValues.size() > 1);
         }
 
         public Term bound(Bound b)
diff --git a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java
index f31196a3c1..3796173f3b 100644
--- a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java
@@ -173,7 +173,7 @@ public class UpdateStatement extends ModificationStatement
         for (CFDefinition.Name name : cfDef.keys.values())
         {
             List<Term> values = processed.get(name.name);
-            if (values == null || values.isEmpty())
+            if (values == null)
                 throw new InvalidRequestException(String.format("Missing mandatory PRIMARY KEY part %s", name));
 
             if (keyBuilder.remainingCount() == 1)
@@ -188,7 +188,7 @@ public class UpdateStatement extends ModificationStatement
             }
             else
             {
-                if (values.size() > 1)
+                if (values.isEmpty() || values.size() > 1)
                     throw new InvalidRequestException("IN is only supported on the last column of the partition key");
                 ByteBuffer val = values.get(0).bindAndGet(variables);
                 if (val == null)