Repository: cassandra Updated Branches: refs/heads/trunk 2c6924b56 -> f87ec773f
Improve error messages for +/- operations on maps and tuples Patch by Alex Petrov; reviewed by Andrés de la Peña for CASSANDRA-13197 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/f87ec773 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/f87ec773 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/f87ec773 Branch: refs/heads/trunk Commit: f87ec773fe1c698d738e9735f6e8ee513c2ba510 Parents: 2c6924b Author: Alex Petrov <oleksandr.pet...@gmail.com> Authored: Tue Apr 4 09:49:14 2017 +0200 Committer: Alex Petrov <oleksandr.pet...@gmail.com> Committed: Tue Apr 4 09:49:14 2017 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../org/apache/cassandra/cql3/Operation.java | 34 ++++++++++++++++---- .../validation/entities/CollectionsTest.java | 30 +++++++++++++++-- .../cql3/validation/entities/TupleTypeTest.java | 10 +++++- 4 files changed, 65 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/f87ec773/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 3632903..3147eb9 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0 + * Improve error messages for +/- operations on maps and tuples (CASSANDRA-13197) * Remove deprecated repair JMX APIs (CASSANDRA-11530) * Fix version check to enable streaming keep-alive (CASSANDRA-12929) * Make it possible to monitor an ideal consistency level separate from actual consistency level (CASSANDRA-13289) http://git-wip-us.apache.org/repos/asf/cassandra/blob/f87ec773/src/java/org/apache/cassandra/cql3/Operation.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/Operation.java b/src/java/org/apache/cassandra/cql3/Operation.java index 8db9306..85214f1 100644 --- a/src/java/org/apache/cassandra/cql3/Operation.java +++ b/src/java/org/apache/cassandra/cql3/Operation.java @@ -300,13 +300,14 @@ public abstract class Operation public Operation prepare(TableMetadata metadata, ColumnMetadata receiver) throws InvalidRequestException { - Term v = value.prepare(metadata.keyspace, receiver); - if (!(receiver.type instanceof CollectionType)) { + if (receiver.type instanceof TupleType) + throw new InvalidRequestException(String.format("Invalid operation (%s) for tuple column %s", toString(receiver), receiver.name)); + if (!(receiver.type instanceof CounterColumnType)) throw new InvalidRequestException(String.format("Invalid operation (%s) for non counter column %s", toString(receiver), receiver.name)); - return new Constants.Adder(receiver, v); + return new Constants.Adder(receiver, value.prepare(metadata.keyspace, receiver)); } else if (!(receiver.type.isMultiCell())) throw new InvalidRequestException(String.format("Invalid operation (%s) for frozen collection column %s", toString(receiver), receiver.name)); @@ -314,11 +315,21 @@ public abstract class Operation switch (((CollectionType)receiver.type).kind) { case LIST: - return new Lists.Appender(receiver, v); + return new Lists.Appender(receiver, value.prepare(metadata.keyspace, receiver)); case SET: - return new Sets.Adder(receiver, v); + return new Sets.Adder(receiver, value.prepare(metadata.keyspace, receiver)); case MAP: - return new Maps.Putter(receiver, v); + Term term; + try + { + term = value.prepare(metadata.keyspace, receiver); + } + catch (InvalidRequestException e) + { + throw new InvalidRequestException(String.format("Value for a map addition has to be a map, but was: '%s'", value)); + } + + return new Maps.Putter(receiver, term); } throw new AssertionError(); } @@ -366,7 +377,16 @@ public abstract class Operation receiver.cfName, receiver.name, SetType.getInstance(((MapType)receiver.type).getKeysType(), false)); - return new Sets.Discarder(receiver, value.prepare(metadata.keyspace, vr)); + Term term; + try + { + term = value.prepare(metadata.keyspace, vr); + } + catch (InvalidRequestException e) + { + throw new InvalidRequestException(String.format("Value for a map substraction has to be a set, but was: '%s'", value)); + } + return new Sets.Discarder(receiver, term); } throw new AssertionError(); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/f87ec773/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java b/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java index 6292308..56ba0a0 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java @@ -115,13 +115,14 @@ public class CollectionsTest extends CQLTester ); execute("UPDATE %s SET s += ? WHERE k = 0", set("v5")); - execute("UPDATE %s SET s += ? WHERE k = 0", set("v6")); + execute("UPDATE %s SET s += {'v6'} WHERE k = 0"); assertRows(execute("SELECT s FROM %s WHERE k = 0"), row(set("v5", "v6", "v7")) ); - execute("UPDATE %s SET s -= ? WHERE k = 0", set("v6", "v5")); + execute("UPDATE %s SET s -= ? WHERE k = 0", set("v5")); + execute("UPDATE %s SET s -= {'v6'} WHERE k = 0"); assertRows(execute("SELECT s FROM %s WHERE k = 0"), row(set("v7")) @@ -144,6 +145,15 @@ public class CollectionsTest extends CQLTester { createTable("CREATE TABLE %s (k int PRIMARY KEY, m map<text, int>)"); + assertInvalidMessage("Value for a map addition has to be a map, but was: '{1}'", + "UPDATE %s SET m = m + {1} WHERE k = 0;"); + assertInvalidMessage("Not enough bytes to read a map", + "UPDATE %s SET m += ? WHERE k = 0", set("v1")); + assertInvalidMessage("Value for a map substraction has to be a set, but was: '{'v1': 1}'", + "UPDATE %s SET m = m - {'v1': 1} WHERE k = 0", map("v1", 1)); + assertInvalidMessage("Unexpected extraneous bytes after set value", + "UPDATE %s SET m -= ? WHERE k = 0", map("v1", 1)); + execute("INSERT INTO %s(k, m) VALUES (0, ?)", map("v1", 1, "v2", 2)); assertRows(execute("SELECT m FROM %s WHERE k = 0"), @@ -193,6 +203,18 @@ public class CollectionsTest extends CQLTester row(map("v5", 5, "v6", 6)) ); + execute("UPDATE %s SET m += {'v7': 7} WHERE k = 0"); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), + row(map("v5", 5, "v6", 6, "v7", 7)) + ); + + execute("UPDATE %s SET m -= {'v7'} WHERE k = 0"); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), + row(map("v5", 5, "v6", 6)) + ); + execute("DELETE m[?] FROM %s WHERE k = 0", "v6"); assertRows(execute("SELECT m FROM %s WHERE k = 0"), @@ -276,6 +298,10 @@ public class CollectionsTest extends CQLTester execute("UPDATE %s SET l = l - ? WHERE k=0", list("v11")); assertRows(execute("SELECT l FROM %s WHERE k = 0"), row((Object) null)); + + execute("UPDATE %s SET l = l + ? WHERE k = 0", list("v1", "v2", "v1", "v2", "v1", "v2")); + execute("UPDATE %s SET l = l - ? WHERE k=0", list("v1", "v2")); + assertRows(execute("SELECT l FROM %s WHERE k = 0"), row((Object) null)); } @Test http://git-wip-us.apache.org/repos/asf/cassandra/blob/f87ec773/test/unit/org/apache/cassandra/cql3/validation/entities/TupleTypeTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/TupleTypeTest.java b/test/unit/org/apache/cassandra/cql3/validation/entities/TupleTypeTest.java index c369ecd..353a40a 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/entities/TupleTypeTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/entities/TupleTypeTest.java @@ -203,4 +203,12 @@ public class TupleTypeTest extends CQLTester assertInvalidMessage("Not enough bytes to read 0th component", "INSERT INTO %s (pk, t) VALUES (?, ?)", 1, Long.MAX_VALUE); } -} + + @Test + public void testTupleModification() throws Throwable + { + createTable("CREATE TABLE %s(pk int PRIMARY KEY, value tuple<int, int>)"); + assertInvalidMessage("Invalid operation (value = value + (1, 1)) for tuple column value", + "UPDATE %s SET value += (1, 1) WHERE k=0;"); + } +} \ No newline at end of file