Repository: cassandra Updated Branches: refs/heads/cassandra-2.2 68398ada7 -> 5a096274d
Remove distinction between non-existing static columns and existing but null in LWTs Path by Alex Petrov; reviewed by Benjamin Lerer for CASSANDRA-9842 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/85f2bbfd Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/85f2bbfd Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/85f2bbfd Branch: refs/heads/cassandra-2.2 Commit: 85f2bbfd1c6803977ecc1c2053527363078bce22 Parents: a56bc16 Author: Alex Petrov <oleksandr.pet...@gmail.com> Authored: Tue Jun 21 16:22:23 2016 +0200 Committer: Benjamin Lerer <b.le...@gmail.com> Committed: Tue Jun 21 16:22:23 2016 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../cql3/statements/ModificationStatement.java | 5 +- .../apache/cassandra/service/StorageProxy.java | 5 +- .../operations/InsertUpdateIfConditionTest.java | 291 ++++++++++++++++++- 4 files changed, 298 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/85f2bbfd/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 64cfdaa..7944967 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 2.1.15 + * Remove distinction between non-existing static columns and existing but null in LWTs (CASSANDRA-9842) * Support mlockall on IBM POWER arch (CASSANDRA-11576) * Cache local ranges when calculating repair neighbors (CASSANDRA-11933) * Allow LWT operation on static column with only partition keys (CASSANDRA-10532) http://git-wip-us.apache.org/repos/asf/cassandra/blob/85f2bbfd/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java index a9f65e1..f84188a 100644 --- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java @@ -697,10 +697,11 @@ public abstract class ModificationStatement implements CQLStatement Row row = readCommand.getRow(keyspace); ColumnFamily current = row.cf; + if (current == null) + current = ArrayBackedSortedColumns.factory.create(metadata); + if (!request.appliesTo(current)) { - if (current == null) - current = ArrayBackedSortedColumns.factory.create(metadata); return current; } http://git-wip-us.apache.org/repos/asf/cassandra/blob/85f2bbfd/src/java/org/apache/cassandra/service/StorageProxy.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/StorageProxy.java b/src/java/org/apache/cassandra/service/StorageProxy.java index 58a2874..845a732 100644 --- a/src/java/org/apache/cassandra/service/StorageProxy.java +++ b/src/java/org/apache/cassandra/service/StorageProxy.java @@ -233,12 +233,15 @@ public class StorageProxy implements StorageProxyMBean ? ConsistencyLevel.LOCAL_QUORUM : ConsistencyLevel.QUORUM); ColumnFamily current = rows.get(0).cf; + if (current == null) + current = ArrayBackedSortedColumns.factory.create(metadata); + if (!request.appliesTo(current)) { Tracing.trace("CAS precondition does not match current values {}", current); // We should not return null as this means success casWriteMetrics.conditionNotMet.inc(); - return current == null ? ArrayBackedSortedColumns.factory.create(metadata) : current; + return current; } // finish the paxos round w/ the desired updates http://git-wip-us.apache.org/repos/asf/cassandra/blob/85f2bbfd/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 05ba09d..900cc7d 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/InsertUpdateIfConditionTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/InsertUpdateIfConditionTest.java @@ -140,7 +140,7 @@ public class InsertUpdateIfConditionTest extends CQLTester @Test public void testConditionalDelete() throws Throwable { - createTable("CREATE TABLE %s (k int PRIMARY KEY, v1 int,)"); + createTable("CREATE TABLE %s (k int PRIMARY KEY, v1 int)"); assertRows(execute("DELETE FROM %s WHERE k=1 IF EXISTS"), row(false)); @@ -981,4 +981,293 @@ public class InsertUpdateIfConditionTest extends CQLTester execute("DROP TYPE IF EXISTS mytype"); assertEmpty(execute("SELECT type_name from system.schema_usertypes where keyspace_name = ? and type_name = ?", KEYSPACE, "mytype")); } + + @Test + public void testConditionalUpdatesOnStaticColumns() throws Throwable + { + createTable("CREATE TABLE %s (a int, b int, s int static, d text, PRIMARY KEY (a, b))"); + + // pre-existing row + execute("INSERT INTO %s (a, b, s, d) values (6, 6, 100, 'a')"); + assertRows(execute("UPDATE %s SET s = 6 WHERE a = 6 IF s = 100"), + row(true)); + assertRows(execute("SELECT * FROM %s WHERE a = 6"), + row(6, 6, 6, "a")); + + // pre-existing row with null in the static column + execute("INSERT INTO %s (a, b, d) values (7, 7, 'a')"); + assertRows(execute("UPDATE %s SET s = 7 WHERE a = 7 IF s = NULL"), + row(true)); + assertRows(execute("SELECT * FROM %s WHERE a = 7"), + row(7, 7, 7, "a")); + + // deleting row before CAS + execute("DELETE FROM %s WHERE a = 8;"); + assertRows(execute("UPDATE %s SET s = 8 WHERE a = 8 IF s = NULL"), + row(true)); + assertRows(execute("SELECT * FROM %s WHERE a = 8"), + row(8, null, 8, null)); + } + + @Test + public void testConditionalUpdatesWithNullValues() throws Throwable + { + createTable("CREATE TABLE %s (a int, b int, s int static, d text, PRIMARY KEY (a, b))"); + + // pre-populate, leave out static column + for (int i = 1; i <= 5; i++) + execute("INSERT INTO %s (a, b) VALUES (?, ?)", i, i); + + conditionalUpdatesWithNonExistingOrNullValues(); + + // rejected: IN doesn't contain null + assertRows(execute("UPDATE %s SET s = 30 WHERE a = 3 IF s IN (10,20,30)"), + row(false)); + assertRows(execute("SELECT * FROM %s WHERE a = 3"), + row(3, 3, null, null)); + + // rejected: comparing number with NULL always returns false + for (String operator: new String[] { ">", "<", ">=", "<=", "="}) + { + assertRows(execute("UPDATE %s SET s = 50 WHERE a = 5 IF s " + operator + " 3"), + row(false)); + assertRows(execute("SELECT * FROM %s WHERE a = 5"), + row(5, 5, null, null)); + } + + } + + @Test + public void testConditionalUpdatesWithNonExistingValues() throws Throwable + { + createTable("CREATE TABLE %s (a int, b int, s int static, d text, PRIMARY KEY (a, b))"); + + conditionalUpdatesWithNonExistingOrNullValues(); + + // rejected: IN doesn't contain null + assertRows(execute("UPDATE %s SET s = 3 WHERE a = 3 IF s IN (10,20,30)"), + row(false)); + assertEmpty(execute("SELECT a, s, d FROM %s WHERE a = 3")); + + // rejected: comparing number with NULL always returns false + for (String operator : new String[]{ ">", "<", ">=", "<=", "=" }) + { + assertRows(execute("UPDATE %s SET s = 50 WHERE a = 5 IF s " + operator + " 3"), + row(false)); + assertEmpty(execute("SELECT * FROM %s WHERE a = 5")); + } + } + + private void conditionalUpdatesWithNonExistingOrNullValues() throws Throwable + { + assertRows(execute("UPDATE %s SET s = 1 WHERE a = 1 IF s = NULL"), + row(true)); + assertRows(execute("SELECT a, s, d FROM %s WHERE a = 1"), + row(1, 1, null)); + + assertRows(execute("UPDATE %s SET s = 2 WHERE a = 2 IF s IN (10,20,NULL)"), + row(true)); + assertRows(execute("SELECT a, s, d FROM %s WHERE a = 2"), + row(2, 2, null)); + + assertRows(execute("UPDATE %s SET s = 4 WHERE a = 4 IF s != 4"), + row(true)); + assertRows(execute("SELECT a, s, d FROM %s WHERE a = 4"), + row(4, 4, null)); + } + + @Test + public void testConditionalUpdatesWithNullValuesWithBatch() throws Throwable + { + createTable("CREATE TABLE %s (a int, b int, s int static, d text, PRIMARY KEY (a, b))"); + + // pre-populate, leave out static column + for (int i = 1; i <= 6; i++) + execute("INSERT INTO %s (a, b) VALUES (?, ?)", i, i); + + testConditionalUpdatesWithNonExistingOrNullValuesWithBatch(); + + // rejected: comparing number with null value always returns false + for (String operator: new String[] { ">", "<", ">=", "<=", "="}) + { + assertRows(execute("BEGIN BATCH\n" + + "INSERT INTO %1$s (a, b, s, d) values (3, 3, 40, 'a');\n" + + "UPDATE %1$s SET s = 30 WHERE a = 3 IF s " + operator + " 5;\n" + + "APPLY BATCH"), + row(false)); + assertRows(execute("SELECT * FROM %s WHERE a = 3"), + row(3, 3, null, null)); + } + + // rejected: IN doesn't contain null + assertRows(execute("BEGIN BATCH\n" + + "INSERT INTO %1$s (a, b, s, d) values (6, 6, 70, 'a');\n" + + "UPDATE %1$s SET s = 60 WHERE a = 6 IF s IN (1,2,3);\n" + + "APPLY BATCH"), + row(false)); + assertRows(execute("SELECT * FROM %s WHERE a = 6"), + row(6, 6, null, null)); + + } + + @Test + public void testConditionalUpdatesWithNonExistingValuesWithBatch() throws Throwable + { + createTable("CREATE TABLE %s (a int, b int, s int static, d text, PRIMARY KEY (a, b))"); + + testConditionalUpdatesWithNonExistingOrNullValuesWithBatch(); + + // rejected: comparing number with non-existing value always returns false + for (String operator: new String[] { ">", "<", ">=", "<=", "="}) + { + assertRows(execute("BEGIN BATCH\n" + + "INSERT INTO %1$s (a, b, s, d) values (3, 3, 3, 'a');\n" + + "UPDATE %1$s SET s = 3 WHERE a = 3 IF s " + operator + " 5;\n" + + "APPLY BATCH"), + row(false)); + assertEmpty(execute("SELECT * FROM %s WHERE a = 3")); + } + + // rejected: IN doesn't contain null + assertRows(execute("BEGIN BATCH\n" + + "INSERT INTO %1$s (a, b, s, d) values (6, 6, 6, 'a');\n" + + "UPDATE %1$s SET s = 7 WHERE a = 6 IF s IN (1,2,3);\n" + + "APPLY BATCH"), + row(false)); + assertEmpty(execute("SELECT * FROM %s WHERE a = 6")); + } + + private void testConditionalUpdatesWithNonExistingOrNullValuesWithBatch() throws Throwable + { + // applied: null is indistiguishable from empty value, lwt condition is executed before INSERT + assertRows(execute("BEGIN BATCH\n" + + "INSERT INTO %1$s (a, b, d) values (2, 2, 'a');\n" + + "UPDATE %1$s SET s = 2 WHERE a = 2 IF s = null;\n" + + "APPLY BATCH"), + row(true)); + assertRows(execute("SELECT * FROM %s WHERE a = 2"), + row(2, 2, 2, "a")); + + // applied: lwt condition is executed before INSERT, update is applied after it + assertRows(execute("BEGIN BATCH\n" + + "INSERT INTO %1$s (a, b, s, d) values (4, 4, 4, 'a');\n" + + "UPDATE %1$s SET s = 5 WHERE a = 4 IF s = null;\n" + + "APPLY BATCH"), + row(true)); + assertRows(execute("SELECT * FROM %s WHERE a = 4"), + row(4, 4, 5, "a")); + + assertRows(execute("BEGIN BATCH\n" + + "INSERT INTO %1$s (a, b, s, d) values (5, 5, 5, 'a');\n" + + "UPDATE %1$s SET s = 6 WHERE a = 5 IF s IN (1,2,null);\n" + + "APPLY BATCH"), + row(true)); + assertRows(execute("SELECT * FROM %s WHERE a = 5"), + row(5, 5, 6, "a")); + + assertRows(execute("BEGIN BATCH\n" + + "INSERT INTO %1$s (a, b, s, d) values (7, 7, 7, 'a');\n" + + "UPDATE %1$s SET s = 8 WHERE a = 7 IF s != 7;\n" + + "APPLY BATCH"), + row(true)); + assertRows(execute("SELECT * FROM %s WHERE a = 7"), + row(7, 7, 8, "a")); + } + + @Test + public void testConditionalDeleteWithNullValues() throws Throwable + { + createTable("CREATE TABLE %s (a int, b int, s1 int static, s2 int static, v int, PRIMARY KEY (a, b))"); + + for (int i = 1; i <= 5; i++) + execute("INSERT INTO %s (a, b, s1, s2, v) VALUES (?, ?, ?, ?, ?)", i, i, i, null, i); + + assertRows(execute("DELETE s1 FROM %s WHERE a = 1 IF s2 = NULL"), + row(true)); + assertRows(execute("SELECT * FROM %s WHERE a = 1"), + row(1, 1, null, null, 1)); + + // rejected: IN doesn't contain null + assertRows(execute("DELETE s1 FROM %s WHERE a = 2 IF s2 IN (10,20,30)"), + row(false, null)); + assertRows(execute("SELECT * FROM %s WHERE a = 2"), + row(2, 2, 2, null, 2)); + + assertRows(execute("DELETE s1 FROM %s WHERE a = 3 IF s2 IN (NULL,20,30)"), + row(true)); + assertRows(execute("SELECT * FROM %s WHERE a = 3"), + row(3, 3, null, null, 3)); + + assertRows(execute("DELETE s1 FROM %s WHERE a = 4 IF s2 != 4"), + row(true)); + assertRows(execute("SELECT * FROM %s WHERE a = 4"), + row(4, 4, null, null, 4)); + + // rejected: comparing number with NULL always returns false + for (String operator : new String[]{ ">", "<", ">=", "<=", "=" }) + { + assertRows(execute("DELETE s1 FROM %s WHERE a = 5 IF s2 " + operator + " 3"), + row(false, null)); + assertRows(execute("SELECT * FROM %s WHERE a = 5"), + row(5, 5, 5, null, 5)); + } + } + + @Test + public void testConditionalDeletesWithNonExistingValuesWithBatch() throws Throwable + { + createTable("CREATE TABLE %s (a int, b int, s1 int static, s2 int static, v int, PRIMARY KEY (a, b))"); + + // applied: null is indistiguishable from empty value, lwt condition is executed before INSERT + assertRows(execute("BEGIN BATCH\n" + + "INSERT INTO %1$s (a, b, s1, v) values (2, 2, 2, 2);\n" + + "DELETE s1 FROM %1$s WHERE a = 2 IF s2 = null;\n" + + "APPLY BATCH"), + row(true)); + assertRows(execute("SELECT * FROM %s WHERE a = 2"), + row(2, 2, null, null, 2)); + + // rejected: comparing number with non-existing value always returns false + for (String operator: new String[] { ">", "<", ">=", "<=", "="}) + { + assertRows(execute("BEGIN BATCH\n" + + "INSERT INTO %1$s (a, b, s1, v) values (3, 3, 3, 3);\n" + + "DELETE s1 FROM %1$s WHERE a = 3 IF s2 " + operator + " 5;\n" + + "APPLY BATCH"), + row(false)); + assertEmpty(execute("SELECT * FROM %s WHERE a = 3")); + } + + // rejected: IN doesn't contain null + assertRows(execute("BEGIN BATCH\n" + + "INSERT INTO %1$s (a, b, s1, v) values (6, 6, 6, 6);\n" + + "DELETE s1 FROM %1$s WHERE a = 6 IF s2 IN (1,2,3);\n" + + "APPLY BATCH"), + row(false)); + assertEmpty(execute("SELECT * FROM %s WHERE a = 6")); + + assertRows(execute("BEGIN BATCH\n" + + "INSERT INTO %1$s (a, b, s1, v) values (4, 4, 4, 4);\n" + + "DELETE s1 FROM %1$s WHERE a = 4 IF s2 = null;\n" + + "APPLY BATCH"), + row(true)); + assertRows(execute("SELECT * FROM %s WHERE a = 4"), + row(4, 4, null, null, 4)); + + assertRows(execute("BEGIN BATCH\n" + + "INSERT INTO %1$s (a, b, s1, v) VALUES (5, 5, 5, 5);\n" + + "DELETE s1 FROM %1$s WHERE a = 5 IF s1 IN (1,2,null);\n" + + "APPLY BATCH"), + row(true)); + assertRows(execute("SELECT * FROM %s WHERE a = 5"), + row(5, 5, null, null, 5)); + + assertRows(execute("BEGIN BATCH\n" + + "INSERT INTO %1$s (a, b, s1, v) values (7, 7, 7, 7);\n" + + "DELETE s1 FROM %1$s WHERE a = 7 IF s2 != 7;\n" + + "APPLY BATCH"), + row(true)); + assertRows(execute("SELECT * FROM %s WHERE a = 7"), + row(7, 7, null, null, 7)); + } }
