Repository: cassandra Updated Branches: refs/heads/cassandra-2.2 e5a553339 -> 70e33d96e
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-2.2 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()); + } + } }
