You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2016/11/07 13:11:16 UTC
[2/6] cassandra git commit: Batch with multiple conditional updates
for the same partition causes AssertionError
Batch with multiple conditional updates for the same partition causes AssertionError
patch by Sylvain Lebresne; reviewed by Benjamin Lerer for CASSANDRA-12867
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/78fdfe23
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/78fdfe23
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/78fdfe23
Branch: refs/heads/cassandra-3.X
Commit: 78fdfe2336048ba37a8b4c321ee4ab5d7bfb1357
Parents: f7aa371
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Nov 2 14:47:42 2016 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Mon Nov 7 14:09:22 2016 +0100
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../cql3/statements/CQL3CasRequest.java | 27 ++++-
.../operations/InsertUpdateIfConditionTest.java | 104 +++++++++++++++++++
3 files changed, 127 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/78fdfe23/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index f708602..1d2c8f3 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
3.0.10
+ * Batch with multiple conditional updates for the same partition causes AssertionError (CASSANDRA-12867)
* Make AbstractReplicationStrategy extendable from outside its package (CASSANDRA-12788)
* Fix CommitLogTest.testDeleteIfNotDirty (CASSANDRA-12854)
* Don't tell users to turn off consistent rangemovements during rebuild. (CASSANDRA-12296)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/78fdfe23/src/java/org/apache/cassandra/cql3/statements/CQL3CasRequest.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/CQL3CasRequest.java b/src/java/org/apache/cassandra/cql3/statements/CQL3CasRequest.java
index cf4110c..d9e8796 100644
--- a/src/java/org/apache/cassandra/cql3/statements/CQL3CasRequest.java
+++ b/src/java/org/apache/cassandra/cql3/statements/CQL3CasRequest.java
@@ -93,12 +93,29 @@ public class CQL3CasRequest implements CASRequest
{
assert condition instanceof ExistCondition || condition instanceof NotExistCondition;
RowCondition previous = getConditionsForRow(clustering);
- if (previous != null && !(previous.getClass().equals(condition.getClass())))
+ if (previous != null)
{
- // these should be prevented by the parser, but it doesn't hurt to check
- throw (previous instanceof NotExistCondition || previous instanceof ExistCondition)
- ? new InvalidRequestException("Cannot mix IF EXISTS and IF NOT EXISTS conditions for the same row")
- : new InvalidRequestException("Cannot mix IF conditions and IF " + (isNotExist ? "NOT " : "") + "EXISTS for the same row");
+ if (previous.getClass().equals(condition.getClass()))
+ {
+ // We can get here if a BATCH has 2 different statements on the same row with the same "exist" condition.
+ // For instance (assuming 'k' is the full PK):
+ // BEGIN BATCH
+ // INSERT INTO t(k, v1) VALUES (0, 'foo') IF NOT EXISTS;
+ // INSERT INTO t(k, v2) VALUES (0, 'bar') IF NOT EXISTS;
+ // APPLY BATCH;
+ // Of course, those can be trivially rewritten by the user as a single INSERT statement, but we still don't
+ // want this to be a problem (see #12867 in particular), so we simply return (the condition itself has
+ // already be set).
+ assert hasExists; // We shouldn't have a previous condition unless hasExists has been set already.
+ return;
+ }
+ else
+ {
+ // these should be prevented by the parser, but it doesn't hurt to check
+ throw (previous instanceof NotExistCondition || previous instanceof ExistCondition)
+ ? new InvalidRequestException("Cannot mix IF EXISTS and IF NOT EXISTS conditions for the same row")
+ : new InvalidRequestException("Cannot mix IF conditions and IF " + (isNotExist ? "NOT " : "") + "EXISTS for the same row");
+ }
}
setConditionsForRow(clustering, condition);
http://git-wip-us.apache.org/repos/asf/cassandra/blob/78fdfe23/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 352100e..40db977 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/operations/InsertUpdateIfConditionTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/operations/InsertUpdateIfConditionTest.java
@@ -1421,4 +1421,108 @@ public class InsertUpdateIfConditionTest extends CQLTester
assertRows(execute("SELECT * FROM %s WHERE a = 7"),
row(7, 7, null, null, 7));
}
+
+ /**
+ * Test for CASSANDRA-12060, using a table without clustering.
+ */
+ @Test
+ public void testMultiExistConditionOnSameRowNoClustering() throws Throwable
+ {
+ createTable("CREATE TABLE %s (k int PRIMARY KEY, v1 text, v2 text)");
+
+ // Multiple inserts on the same row with not exist conditions
+ assertRows(execute("BEGIN BATCH "
+ + "INSERT INTO %1$s (k, v1) values (0, 'foo') IF NOT EXISTS; "
+ + "INSERT INTO %1$s (k, v2) values (0, 'bar') IF NOT EXISTS; "
+ + "APPLY BATCH"),
+ row(true));
+
+ assertRows(execute("SELECT * FROM %s WHERE k = 0"), row(0, "foo", "bar"));
+
+ // Same, but both insert on the same column: doing so would almost surely be a user error, but that's the
+ // original case reported in #12867, so being thorough.
+ assertRows(execute("BEGIN BATCH "
+ + "INSERT INTO %1$s (k, v1) values (1, 'foo') IF NOT EXISTS; "
+ + "INSERT INTO %1$s (k, v1) values (1, 'bar') IF NOT EXISTS; "
+ + "APPLY BATCH"),
+ row(true));
+
+ // As all statement gets the same timestamp, the biggest value ends up winning, so that's "foo"
+ assertRows(execute("SELECT * FROM %s WHERE k = 1"), row(1, "foo", null));
+
+ // Multiple deletes on the same row with exists conditions (note that this is somewhat non-sensical, one of the
+ // delete is redundant, we're just checking it doesn't break something)
+ assertRows(execute("BEGIN BATCH "
+ + "DELETE FROM %1$s WHERE k = 0 IF EXISTS; "
+ + "DELETE FROM %1$s WHERE k = 0 IF EXISTS; "
+ + "APPLY BATCH"),
+ row(true));
+
+ assertEmpty(execute("SELECT * FROM %s WHERE k = 0"));
+
+ // Validate we can't mix different type of conditions however
+ assertInvalidMessage("Cannot mix IF EXISTS and IF NOT EXISTS conditions for the same row",
+ "BEGIN BATCH "
+ + "INSERT INTO %1$s (k, v1) values (1, 'foo') IF NOT EXISTS; "
+ + "DELETE FROM %1$s WHERE k = 1 IF EXISTS; "
+ + "APPLY BATCH");
+
+ assertInvalidMessage("Cannot mix IF conditions and IF NOT EXISTS for the same row",
+ "BEGIN BATCH "
+ + "INSERT INTO %1$s (k, v1) values (1, 'foo') IF NOT EXISTS; "
+ + "UPDATE %1$s SET v2 = 'bar' WHERE k = 1 IF v1 = 'foo'; "
+ + "APPLY BATCH");
+ }
+
+ /**
+ * Test for CASSANDRA-12060, using a table with clustering.
+ */
+ @Test
+ public void testMultiExistConditionOnSameRowClustering() throws Throwable
+ {
+ createTable("CREATE TABLE %s (k int, t int, v1 text, v2 text, PRIMARY KEY (k, t))");
+
+ // Multiple inserts on the same row with not exist conditions
+ assertRows(execute("BEGIN BATCH "
+ + "INSERT INTO %1$s (k, t, v1) values (0, 0, 'foo') IF NOT EXISTS; "
+ + "INSERT INTO %1$s (k, t, v2) values (0, 0, 'bar') IF NOT EXISTS; "
+ + "APPLY BATCH"),
+ row(true));
+
+ assertRows(execute("SELECT * FROM %s WHERE k = 0"), row(0, 0, "foo", "bar"));
+
+ // Same, but both insert on the same column: doing so would almost surely be a user error, but that's the
+ // original case reported in #12867, so being thorough.
+ assertRows(execute("BEGIN BATCH "
+ + "INSERT INTO %1$s (k, t, v1) values (1, 0, 'foo') IF NOT EXISTS; "
+ + "INSERT INTO %1$s (k, t, v1) values (1, 0, 'bar') IF NOT EXISTS; "
+ + "APPLY BATCH"),
+ row(true));
+
+ // As all statement gets the same timestamp, the biggest value ends up winning, so that's "foo"
+ assertRows(execute("SELECT * FROM %s WHERE k = 1"), row(1, 0, "foo", null));
+
+ // Multiple deletes on the same row with exists conditions (note that this is somewhat non-sensical, one of the
+ // delete is redundant, we're just checking it doesn't break something)
+ assertRows(execute("BEGIN BATCH "
+ + "DELETE FROM %1$s WHERE k = 0 AND t = 0 IF EXISTS; "
+ + "DELETE FROM %1$s WHERE k = 0 AND t = 0 IF EXISTS; "
+ + "APPLY BATCH"),
+ row(true));
+
+ assertEmpty(execute("SELECT * FROM %s WHERE k = 0"));
+
+ // Validate we can't mix different type of conditions however
+ assertInvalidMessage("Cannot mix IF EXISTS and IF NOT EXISTS conditions for the same row",
+ "BEGIN BATCH "
+ + "INSERT INTO %1$s (k, t, v1) values (1, 0, 'foo') IF NOT EXISTS; "
+ + "DELETE FROM %1$s WHERE k = 1 AND t = 0 IF EXISTS; "
+ + "APPLY BATCH");
+
+ assertInvalidMessage("Cannot mix IF conditions and IF NOT EXISTS for the same row",
+ "BEGIN BATCH "
+ + "INSERT INTO %1$s (k, t, v1) values (1, 0, 'foo') IF NOT EXISTS; "
+ + "UPDATE %1$s SET v2 = 'bar' WHERE k = 1 AND t = 0 IF v1 = 'foo'; "
+ + "APPLY BATCH");
+ }
}