Repository: cassandra
Updated Branches:
  refs/heads/cassandra-3.11 84d836137 -> 078a84154


Fix handling of nulls and unsets in IN conditions

patch by Benjamin Lerer; reviewed by Alex Petrov for CASSANDRA-12981


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/70e33d96
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/70e33d96
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/70e33d96

Branch: refs/heads/cassandra-3.11
Commit: 70e33d96e1f1236788afb50c1f02fbc64d760281
Parents: e5a5533
Author: Benjamin Lerer <b.le...@gmail.com>
Authored: Fri Jan 27 15:17:53 2017 +0100
Committer: Benjamin Lerer <b.le...@gmail.com>
Committed: Fri Jan 27 15:17:53 2017 +0100

----------------------------------------------------------------------
 CHANGES.txt                                     |   1 +
 .../apache/cassandra/cql3/ColumnCondition.java  |  62 ++++++-
 .../operations/InsertUpdateIfConditionTest.java | 161 ++++++++++++++++++-
 3 files changed, 214 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/70e33d96/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 4f769a1..c5e5335 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 2.2.9
+ * Fix handling of nulls and unsets in IN conditions (CASSANDRA-12981) 
  * Remove support for non-JavaScript UDFs (CASSANDRA-12883)
  * Fix DynamicEndpointSnitch noop in multi-datacenter situations 
(CASSANDRA-13074)
  * cqlsh copy-from: encode column names to avoid primary key parsing errors 
(CASSANDRA-12909)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/70e33d96/src/java/org/apache/cassandra/cql3/ColumnCondition.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/ColumnCondition.java 
b/src/java/org/apache/cassandra/cql3/ColumnCondition.java
index c7b5ddb..3412e71 100644
--- a/src/java/org/apache/cassandra/cql3/ColumnCondition.java
+++ b/src/java/org/apache/cassandra/cql3/ColumnCondition.java
@@ -25,6 +25,7 @@ import com.google.common.collect.Iterables;
 import com.google.common.collect.Iterators;
 
 import org.apache.cassandra.config.ColumnDefinition;
+import org.apache.cassandra.cql3.Term.Terminal;
 import org.apache.cassandra.cql3.functions.Function;
 import org.apache.cassandra.db.Cell;
 import org.apache.cassandra.db.ColumnFamily;
@@ -267,12 +268,26 @@ public class ColumnCondition
             assert !(column.type instanceof CollectionType) && 
condition.collectionElement == null;
             assert condition.operator == Operator.IN;
             if (condition.inValues == null)
-                this.inValues = ((Lists.Value) 
condition.value.bind(options)).getElements();
+            {
+                Terminal terminal = condition.value.bind(options);
+
+                if (terminal == null)
+                    throw new InvalidRequestException("Invalid null list in IN 
condition");
+
+                if (terminal == Constants.UNSET_VALUE)
+                    throw new InvalidRequestException("Invalid 'unset' value 
in condition");
+
+                this.inValues = ((Lists.Value) terminal).getElements();
+            }
             else
             {
                 this.inValues = new ArrayList<>(condition.inValues.size());
                 for (Term value : condition.inValues)
-                    this.inValues.add(value.bindAndGet(options));
+                {
+                    ByteBuffer buffer = value.bindAndGet(options);
+                    if (buffer != ByteBufferUtil.UNSET_BYTE_BUFFER)
+                        this.inValues.add(value.bindAndGet(options));
+                }
             }
         }
 
@@ -378,12 +393,22 @@ public class ColumnCondition
             this.collectionElement = 
condition.collectionElement.bindAndGet(options);
 
             if (condition.inValues == null)
-                this.inValues = ((Lists.Value) 
condition.value.bind(options)).getElements();
+            {
+                Terminal terminal = condition.value.bind(options);
+                if (terminal == Constants.UNSET_VALUE)
+                    throw new InvalidRequestException("Invalid 'unset' value 
in condition");
+                this.inValues = ((Lists.Value) terminal).getElements();
+            }
             else
             {
                 this.inValues = new ArrayList<>(condition.inValues.size());
                 for (Term value : condition.inValues)
-                    this.inValues.add(value.bindAndGet(options));
+                {
+                    ByteBuffer buffer = value.bindAndGet(options);
+                    // We want to ignore unset values
+                    if (buffer != ByteBufferUtil.UNSET_BYTE_BUFFER)
+                        this.inValues.add(buffer);
+                }
             }
         }
 
@@ -642,11 +667,22 @@ public class ColumnCondition
                 // We have a list of serialized collections that need to be 
deserialized for later comparisons
                 CollectionType collectionType = (CollectionType) column.type;
                 Lists.Marker inValuesMarker = (Lists.Marker) condition.value;
+                Terminal terminal = inValuesMarker.bind(options);
+
+                if (terminal == null)
+                    throw new InvalidRequestException("Invalid null list in IN 
condition");
+
+                if (terminal == Constants.UNSET_VALUE)
+                    throw new InvalidRequestException("Invalid 'unset' value 
in condition");
+
                 if (column.type instanceof ListType)
                 {
                     ListType deserializer = 
ListType.getInstance(collectionType.valueComparator(), false);
-                    for (ByteBuffer buffer : 
((Lists.Value)inValuesMarker.bind(options)).elements)
+                    for (ByteBuffer buffer : ((Lists.Value) terminal).elements)
                     {
+                        if (buffer == ByteBufferUtil.UNSET_BYTE_BUFFER)
+                            continue;
+
                         if (buffer == null)
                             this.inValues.add(null);
                         else
@@ -656,8 +692,11 @@ public class ColumnCondition
                 else if (column.type instanceof MapType)
                 {
                     MapType deserializer = 
MapType.getInstance(collectionType.nameComparator(), 
collectionType.valueComparator(), false);
-                    for (ByteBuffer buffer : 
((Lists.Value)inValuesMarker.bind(options)).elements)
+                    for (ByteBuffer buffer : ((Lists.Value) terminal).elements)
                     {
+                        if (buffer == ByteBufferUtil.UNSET_BYTE_BUFFER)
+                            continue;
+
                         if (buffer == null)
                             this.inValues.add(null);
                         else
@@ -667,8 +706,11 @@ public class ColumnCondition
                 else if (column.type instanceof SetType)
                 {
                     SetType deserializer = 
SetType.getInstance(collectionType.valueComparator(), false);
-                    for (ByteBuffer buffer : 
((Lists.Value)inValuesMarker.bind(options)).elements)
+                    for (ByteBuffer buffer : ((Lists.Value) terminal).elements)
                     {
+                        if (buffer == ByteBufferUtil.UNSET_BYTE_BUFFER)
+                            continue;
+
                         if (buffer == null)
                             this.inValues.add(null);
                         else
@@ -679,7 +721,11 @@ public class ColumnCondition
             else
             {
                 for (Term value : condition.inValues)
-                    this.inValues.add(value.bind(options));
+                {
+                    Terminal terminal = value.bind(options);
+                    if (terminal != Constants.UNSET_VALUE)
+                        this.inValues.add(terminal);
+                }
             }
         }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/70e33d96/test/unit/org/apache/cassandra/cql3/validation/operations/InsertUpdateIfConditionTest.java
----------------------------------------------------------------------
diff --git 
a/test/unit/org/apache/cassandra/cql3/validation/operations/InsertUpdateIfConditionTest.java
 
b/test/unit/org/apache/cassandra/cql3/validation/operations/InsertUpdateIfConditionTest.java
index 4a209e6..6396727 100644
--- 
a/test/unit/org/apache/cassandra/cql3/validation/operations/InsertUpdateIfConditionTest.java
+++ 
b/test/unit/org/apache/cassandra/cql3/validation/operations/InsertUpdateIfConditionTest.java
@@ -18,6 +18,9 @@
 
 package org.apache.cassandra.cql3.validation.operations;
 
+import java.nio.ByteBuffer;
+import java.util.List;
+
 import org.junit.Test;
 
 import org.apache.cassandra.cql3.CQLTester;
@@ -135,6 +138,7 @@ public class InsertUpdateIfConditionTest extends CQLTester
         assertRows(execute("UPDATE %s SET v2 = 'bar' WHERE k = 0 IF v1 IN (?, 
?, ?)", 0, 1, 2), row(true));
         assertRows(execute("UPDATE %s SET v2 = 'bar' WHERE k = 0 IF v1 IN ?", 
list(142, 276)), row(false, 2));
         assertRows(execute("UPDATE %s SET v2 = 'bar' WHERE k = 0 IF v1 IN 
()"), row(false, 2));
+        assertRows(execute("UPDATE %s SET v2 = 'bar' WHERE k = 0 IF v1 IN (?, 
?)", unset(), 1), row(false, 2));
 
         assertInvalidMessage("Invalid 'unset' value in condition",
                              "UPDATE %s SET v1 = 3, v2 = 'bar' WHERE k = 0 IF 
v1 < ?", unset());
@@ -146,8 +150,6 @@ public class InsertUpdateIfConditionTest extends CQLTester
                              "UPDATE %s SET v1 = 3, v2 = 'bar' WHERE k = 0 IF 
v1 >= ?", unset());
         assertInvalidMessage("Invalid 'unset' value in condition",
                              "UPDATE %s SET v1 = 3, v2 = 'bar' WHERE k = 0 IF 
v1 != ?", unset());
-        assertInvalidMessage("Invalid 'unset' value in condition",
-                             "UPDATE %s SET v1 = 3, v2 = 'bar' WHERE k = 0 IF 
v1 IN (?, ?)", unset(), 1);
     }
 
     /**
@@ -1303,4 +1305,159 @@ public class InsertUpdateIfConditionTest extends 
CQLTester
         assertRows(execute("SELECT * FROM %s WHERE a = 7"),
                    row(7, 7, null, null, 7));
     }
+
+    @Test
+    public void testInMarkerWithUDTs() throws Throwable
+    {
+        String typename = createType("CREATE TYPE %s (a int, b text)");
+        String myType = KEYSPACE + '.' + typename;
+
+            createTable("CREATE TABLE %s (k int PRIMARY KEY, v frozen<" + 
myType + "> )");
+
+            Object v = userType(0, "abc");
+            execute("INSERT INTO %s (k, v) VALUES (?, ?)", 0, v);
+
+            // Does not apply
+            assertRows(execute("UPDATE %s SET v = {a: 0, b: 'bc'} WHERE k = 0 
IF v IN (?, ?)", userType(1, "abc"), userType(0, "ac")),
+                       row(false, v));
+            assertRows(execute("UPDATE %s SET v = {a: 0, b: 'bc'} WHERE k = 0 
IF v IN (?, ?)", userType(1, "abc"), null),
+                       row(false, v));
+            assertRows(execute("UPDATE %s SET v = {a: 0, b: 'bc'} WHERE k = 0 
IF v IN (?, ?)", userType(1, "abc"), unset()),
+                       row(false, v));
+            assertRows(execute("UPDATE %s SET v = {a: 0, b: 'bc'} WHERE k = 0 
IF v IN (?, ?)", null, null),
+                       row(false, v));
+            assertRows(execute("UPDATE %s SET v = {a: 0, b: 'bc'} WHERE k = 0 
IF v IN (?, ?)", unset(), unset()),
+                       row(false, v));
+            assertRows(execute("UPDATE %s SET v = {a: 0, b: 'bc'} WHERE k = 0 
IF v IN ?", list(userType(1, "abc"), userType(0, "ac"))),
+                       row(false, v));
+
+            // Does apply
+            assertRows(execute("UPDATE %s SET v = {a: 0, b: 'bc'} WHERE k = 0 
IF v IN (?, ?)", userType(0, "abc"), userType(0, "ac")),
+                       row(true));
+            assertRows(execute("UPDATE %s SET v = {a: 1, b: 'bc'} WHERE k = 0 
IF v IN (?, ?)", userType(0, "bc"), null),
+                       row(true));
+            assertRows(execute("UPDATE %s SET v = {a: 1, b: 'ac'} WHERE k = 0 
IF v IN (?, ?, ?)", userType(0, "bc"), unset(), userType(1, "bc")),
+                       row(true));
+            assertRows(execute("UPDATE %s SET v = {a: 0, b: 'abc'} WHERE k = 0 
IF v IN ?", list(userType(1, "ac"), userType(0, "ac"))),
+                       row(true));
+
+            assertInvalidMessage("Invalid null list in IN condition",
+                                 "UPDATE %s SET v = {a: 0, b: 'bc'} WHERE k = 
0 IF v IN ?", (List<ByteBuffer>) null);
+            assertInvalidMessage("Invalid 'unset' value in condition",
+                                 "UPDATE %s SET v = {a: 0, b: 'bc'} WHERE k = 
0 IF v IN ?", unset());
+    }
+
+    @Test
+    public void testInMarkerWithLists() throws Throwable
+    {
+        for (boolean frozen : new boolean[]{false, true})
+        {
+            createTable(String.format("CREATE TABLE %%s (k int PRIMARY KEY, l 
%s)",
+                                      frozen
+                                      ? "frozen<list<text>>"
+                                      : "list<text>"));
+
+            execute("INSERT INTO %s(k, l) VALUES (0, ['foo', 'bar', 
'foobar'])");
+
+            // Does not apply
+            assertRows(execute("UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l IN (?, ?)", list("foo", "foobar"), list("bar", "foobar")),
+                       row(false, list("foo", "bar", "foobar")));
+            assertRows(execute("UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l IN (?, ?)", list("foo", "foobar"), null),
+                       row(false, list("foo", "bar", "foobar")));
+            assertRows(execute("UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l IN (?, ?)", list("foo", "foobar"), unset()),
+                       row(false, list("foo", "bar", "foobar")));
+            assertRows(execute("UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l IN (?, ?)", null, null),
+                       row(false, list("foo", "bar", "foobar")));
+            assertRows(execute("UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l IN ?", list(list("foo", "foobar"), list("bar", "foobar"))),
+                       row(false, list("foo", "bar", "foobar")));
+            assertRows(execute("UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l[?] IN ?", 1, list("foo", "foobar")),
+                       row(false, list("foo", "bar", "foobar")));
+            assertRows(execute("UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l[?] IN (?, ?)", 1, "foo", "foobar"),
+                       row(false, list("foo", "bar", "foobar")));
+            assertRows(execute("UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l[?] IN (?, ?)", 1, "foo", null),
+                       row(false, list("foo", "bar", "foobar")));
+            assertRows(execute("UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l[?] IN (?, ?)", 1, "foo", unset()),
+                       row(false, list("foo", "bar", "foobar")));
+
+            // Does apply
+            assertRows(execute("UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l IN (?, ?)", list("foo", "bar", "foobar"), list("bar", "foobar")),
+                       row(true));
+            assertRows(execute("UPDATE %s SET l = ['foo', 'foobar'] WHERE k = 
0 IF l IN (?, ?, ?)", list("foo", "bar", "foobar"), null, list("foo", "bar")),
+                       row(true));
+            assertRows(execute("UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l IN (?, ?, ?)", list("foo", "bar", "foobar"), unset(), list("foo", 
"foobar")),
+                       row(true));
+            assertRows(execute("UPDATE %s SET l = ['foo', 'foobar'] WHERE k = 
0 IF l IN (?, ?)", list("bar", "foobar"), list("foo", "bar")),
+                       row(true));
+            assertRows(execute("UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l[?] IN ?", 1, list("bar", "foobar")),
+                       row(true));
+            assertRows(execute("UPDATE %s SET l = ['foo', 'foobar'] WHERE k = 
0 IF l[?] IN (?, ?, ?)", 1, "bar", null, "foobar"),
+                       row(true));
+            assertRows(execute("UPDATE %s SET l = ['foo', 'foobar'] WHERE k = 
0 IF l[?] IN (?, ?, ?)", 1, "bar", unset(), "foobar"),
+                       row(true));
+
+            assertInvalidMessage("Invalid null list in IN condition",
+                                 "UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l IN ?", (List<ByteBuffer>) null);
+            assertInvalidMessage("Invalid 'unset' value in condition",
+                                 "UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l IN ?", unset());
+            assertInvalidMessage("Invalid 'unset' value in condition",
+                                 "UPDATE %s SET l = ['foo', 'bar'] WHERE k = 0 
IF l[?] IN ?", 1, unset());
+        }
+    }
+
+    @Test
+    public void testInMarkerWithMaps() throws Throwable
+    {
+        for (boolean frozen : new boolean[] {false, true})
+        {
+            createTable(String.format("CREATE TABLE %%s (k int PRIMARY KEY, m 
%s)",
+                                      frozen
+                                      ? "frozen<map<text, text>>"
+                                      : "map<text, text>"));
+
+            execute("INSERT INTO %s (k, m) VALUES (0, {'foo' : 'bar'})");
+
+            // Does not apply
+            assertRows(execute("UPDATE %s SET m = {'foo' : 'foobar'} WHERE k = 
0 IF m IN (?, ?)", map("foo", "foobar"), map("bar", "foobar")),
+                       row(false, map("foo", "bar")));
+            assertRows(execute("UPDATE %s SET  m = {'foo' : 'foobar'} WHERE k 
= 0 IF m IN (?, ?)", map("foo", "foobar"), null),
+                       row(false, map("foo", "bar")));
+            assertRows(execute("UPDATE %s SET  m = {'foo' : 'foobar'} WHERE k 
= 0 IF m IN (?, ?)", map("foo", "foobar"), unset()),
+                       row(false, map("foo", "bar")));
+            assertRows(execute("UPDATE %s SET  m = {'foo' : 'foobar'} WHERE k 
= 0 IF m IN (?, ?)", null, null),
+                       row(false, map("foo", "bar")));
+            assertRows(execute("UPDATE %s SET  m = {'foo' : 'foobar'} WHERE k 
= 0 IF m IN ?", list(map("foo", "foobar"), map("bar", "foobar"))),
+                       row(false, map("foo", "bar")));
+            assertRows(execute("UPDATE %s SET  m = {'foo' : 'foobar'} WHERE k 
= 0 IF m[?] IN ?", "foo", list("foo", "foobar")),
+                       row(false, map("foo", "bar")));
+            assertRows(execute("UPDATE %s SET  m = {'foo' : 'foobar'} WHERE k 
= 0 IF m[?] IN (?, ?)", "foo", "foo", "foobar"),
+                       row(false, map("foo", "bar")));
+            assertRows(execute("UPDATE %s SET  m = {'foo' : 'foobar'} WHERE k 
= 0 IF m[?] IN (?, ?)", "foo", "foo", null),
+                       row(false, map("foo", "bar")));
+            assertRows(execute("UPDATE %s SET  m = {'foo' : 'foobar'} WHERE k 
= 0 IF m[?] IN (?, ?)", "foo", "foo", unset()),
+                       row(false, map("foo", "bar")));
+
+            // Does apply
+            assertRows(execute("UPDATE %s SET m = {'foo' : 'foobar'} WHERE k = 
0 IF m IN (?, ?)", map("foo", "foobar"), map("foo", "bar")),
+                       row(true));
+            assertRows(execute("UPDATE %s SET m = {'foo' : 'bar'} WHERE k = 0 
IF m IN (?, ?, ?)", map("bar", "foobar"), null, map("foo", "foobar")),
+                       row(true));
+            assertRows(execute("UPDATE %s SET m = {'foo' : 'bar'} WHERE k = 0 
IF m IN (?, ?, ?)", map("bar", "foobar"), unset(), map("foo", "bar")),
+                       row(true));
+            assertRows(execute("UPDATE %s SET m = {'foo' : 'foobar'} WHERE k = 
0 IF m IN ?", list(map("foo", "bar"), map("bar", "foobar"))),
+                       row(true));
+            assertRows(execute("UPDATE %s SET m = {'foo' : 'bar'} WHERE k = 0 
IF m[?] IN ?", "foo", list("bar", "foobar")),
+                       row(true));
+            assertRows(execute("UPDATE %s SET m = {'foo' : 'foobar'} WHERE k = 
0 IF m[?] IN (?, ?, ?)", "foo", "bar", null, "foobar"),
+                       row(true));
+            assertRows(execute("UPDATE %s SET m = {'foo' : 'foobar'} WHERE k = 
0 IF m[?] IN (?, ?, ?)", "foo", "bar", unset(), "foobar"),
+                       row(true));
+
+            assertInvalidMessage("Invalid null list in IN condition",
+                                 "UPDATE %s SET  m = {'foo' : 'foobar'} WHERE 
k = 0 IF m IN ?", (List<ByteBuffer>) null);
+            assertInvalidMessage("Invalid 'unset' value in condition",
+                                 "UPDATE %s SET  m = {'foo' : 'foobar'} WHERE 
k = 0 IF m IN ?", unset());
+            assertInvalidMessage("Invalid 'unset' value in condition",
+                                 "UPDATE %s SET  m = {'foo' : 'foobar'} WHERE 
k = 0 IF m[?] IN ?", "foo", unset());
+        }
+    }
 }

Reply via email to