Repository: cassandra
Updated Branches:
  refs/heads/trunk 325d70d5a -> 8aec742a1


Forbid re-adding static columns as regular and vice versa

patch by Aleksey Yeschenko; reviwed by Benedict Elliott Smith for
CASSANDRA-14913


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

Branch: refs/heads/trunk
Commit: 8aec742a1f86a825028f4a1267e111cd1a4aea40
Parents: 325d70d
Author: Aleksey Yeshchenko <alek...@apple.com>
Authored: Mon Nov 26 16:26:33 2018 +0000
Committer: Aleksey Yeshchenko <alek...@apple.com>
Committed: Tue Nov 27 13:56:55 2018 +0000

----------------------------------------------------------------------
 CHANGES.txt                                     |   1 +
 .../statements/schema/AlterTableStatement.java  |  10 +-
 .../DistributedReadWritePathTest.java           | 173 -------------------
 .../cql3/validation/operations/AlterTest.java   |  15 ++
 4 files changed, 25 insertions(+), 174 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/8aec742a/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 362677a..cda1b8b 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 4.0
+ * Forbid re-adding static columns as regular and vice versa (CASSANDRA-14913)
  * Audit log allows system keyspaces to be audited via configuration options 
(CASSANDRA-14498)
  * Lower default chunk_length_in_kb from 64kb to 16kb (CASSANDRA-13241)
  * Startup checker should wait for count rather than percentage 
(CASSANDRA-14297)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/8aec742a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java
----------------------------------------------------------------------
diff --git 
a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java 
b/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java
index 5044119..c348cc4 100644
--- 
a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java
+++ 
b/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java
@@ -168,12 +168,20 @@ public abstract class AlterTableStatement extends 
AlterSchemaStatement
                 // columns is pushed deeper down the line. The latter would 
still be problematic in cases of schema races.
                 if (!droppedColumn.type.isValueCompatibleWith(type))
                 {
-                    throw ire("Cannot re-add a previously dropped column '%s' 
of type %s, incompatible with previous type %s",
+                    throw ire("Cannot re-add previously dropped column '%s' of 
type %s, incompatible with previous type %s",
                               name,
                               type.asCQL3Type(),
                               droppedColumn.type.asCQL3Type());
                 }
 
+                if (droppedColumn.isStatic() != isStatic)
+                {
+                    throw ire("Cannot re-add previously dropped column '%s' of 
kind %s, incompatible with previous kind %s",
+                              name,
+                              isStatic ? ColumnMetadata.Kind.STATIC : 
ColumnMetadata.Kind.REGULAR,
+                              droppedColumn.kind);
+                }
+
                 // Cannot re-add a dropped counter column. See #7831.
                 if (table.isCounter())
                     throw ire("Cannot re-add previously dropped counter column 
%s", name);

http://git-wip-us.apache.org/repos/asf/cassandra/blob/8aec742a/test/distributed/org/apache/cassandra/distributed/DistributedReadWritePathTest.java
----------------------------------------------------------------------
diff --git 
a/test/distributed/org/apache/cassandra/distributed/DistributedReadWritePathTest.java
 
b/test/distributed/org/apache/cassandra/distributed/DistributedReadWritePathTest.java
index a61c8af..d03ef4f 100644
--- 
a/test/distributed/org/apache/cassandra/distributed/DistributedReadWritePathTest.java
+++ 
b/test/distributed/org/apache/cassandra/distributed/DistributedReadWritePathTest.java
@@ -172,177 +172,4 @@ public class DistributedReadWritePathTest extends 
DistributedTestBase
             Assert.assertTrue(thrown.getCause().getMessage().contains("Unknown 
column v2 during deserialization"));
         }
     }
-
-    @Test
-    public void reAddColumnAsStatic() throws Throwable
-    {
-        try (TestCluster cluster = createCluster(3))
-        {
-            cluster.schemaChange("CREATE TABLE " + KEYSPACE + ".tbl (pk int, 
ck int, v1 int, PRIMARY KEY (pk, ck))");
-
-            for (int i = 1; i <= 3; i++)
-            {
-                cluster.coordinator().execute("INSERT INTO " + KEYSPACE + 
".tbl (pk, ck, v1) VALUES (?, ?, ?)",
-                                              ConsistencyLevel.ALL,
-                                              1, i, i);
-            }
-
-            // Drop column
-            cluster.schemaChange("ALTER TABLE " + KEYSPACE + ".tbl DROP v1");
-
-            assertRows(cluster.coordinator().execute("SELECT * FROM " + 
KEYSPACE + ".tbl WHERE pk = 1",
-                                                     ConsistencyLevel.ALL),
-                       row(1, 1),
-                       row(1, 2),
-                       row(1, 3));
-
-            // Drop column
-            cluster.schemaChange("ALTER TABLE " + KEYSPACE + ".tbl ADD v1 int 
static");
-
-            assertRows(cluster.coordinator().execute("SELECT * FROM " + 
KEYSPACE + ".tbl WHERE pk = 1",
-                                                     ConsistencyLevel.ALL),
-                       row(1, 1, null),
-                       row(1, 2, null),
-                       row(1, 3, null));
-
-            cluster.coordinator().execute("INSERT INTO " + KEYSPACE + ".tbl 
(pk, v1) VALUES (?, ?)",
-                                          ConsistencyLevel.ALL,
-                                          1, 1);
-
-            assertRows(cluster.coordinator().execute("SELECT * FROM " + 
KEYSPACE + ".tbl WHERE pk = 1",
-                                                     ConsistencyLevel.ALL),
-                       row(1, 1, 1),
-                       row(1, 2, 1),
-                       row(1, 3, 1));
-        }
-    }
-
-    @Test
-    public void reAddColumnAsStaticDisagreementCoordinatorSide() throws 
Throwable
-    {
-        try (TestCluster cluster = createCluster(3))
-        {
-            cluster.schemaChange("CREATE TABLE " + KEYSPACE + ".tbl (pk int, 
ck int, v1 int, PRIMARY KEY (pk, ck))");
-
-            for (int i = 1; i <= 3; i++)
-            {
-                cluster.coordinator().execute("INSERT INTO " + KEYSPACE + 
".tbl (pk, ck, v1) VALUES (?, ?, ?)",
-                                              ConsistencyLevel.ALL,
-                                              1, i, i);
-            }
-
-            // Drop column
-            cluster.schemaChange("ALTER TABLE " + KEYSPACE + ".tbl DROP v1", 
1);
-
-            Exception thrown = null;
-            try
-            {
-                cluster.coordinator().execute("SELECT * FROM " + KEYSPACE + 
".tbl WHERE pk = 1",
-                                              ConsistencyLevel.ALL);
-            }
-            catch (Exception e)
-            {
-                thrown = e;
-            }
-
-            Assert.assertTrue(thrown.getCause().getMessage().contains("[v1] is 
not a subset of"));
-
-            cluster.schemaChange("ALTER TABLE " + KEYSPACE + ".tbl ADD v1 int 
static", 1);
-
-            try
-            {
-                cluster.coordinator().execute("SELECT * FROM " + KEYSPACE + 
".tbl WHERE pk = 1",
-                                              ConsistencyLevel.ALL);
-            }
-            catch (Exception e)
-            {
-                thrown = e;
-            }
-
-            Assert.assertTrue(thrown.getCause().getMessage().contains("[v1] is 
not a subset of"));
-        }
-    }
-
-    @Test
-    public void reAddColumnAsStaticDisagreementReplicaSide() throws Throwable
-    {
-        try (TestCluster cluster = createCluster(2))
-        {
-            cluster.schemaChange("CREATE TABLE " + KEYSPACE + ".tbl (pk int, 
ck int, v1 int, PRIMARY KEY (pk, ck)) WITH read_repair='blocking'");
-
-            for (int i = 1; i <= 3; i++)
-            {
-                cluster.coordinator().execute("INSERT INTO " + KEYSPACE + 
".tbl (pk, ck, v1) VALUES (?, ?, ?)",
-                                              ConsistencyLevel.ALL,
-                                              1, i, i);
-            }
-
-            // Drop column on the replica
-            cluster.schemaChange("ALTER TABLE " + KEYSPACE + ".tbl DROP v1", 
2);
-
-            // Columns are going to be read and read-repaired as long as 
they're available
-            assertRows(cluster.coordinator().execute("SELECT * FROM " + 
KEYSPACE + ".tbl WHERE pk = 1",
-                                                     ConsistencyLevel.ALL),
-                       row(1, 1, 1),
-                       row(1, 2, 2),
-                       row(1, 3, 3));
-
-            assertRows(cluster.get(2).executeInternal("SELECT * FROM " + 
KEYSPACE + ".tbl WHERE pk = 1"),
-                       row(1, 1),
-                       row(1, 2),
-                       row(1, 3));
-
-            // Re-add as static on the replica
-            cluster.schemaChange("ALTER TABLE " + KEYSPACE + ".tbl ADD v1 int 
static", 2);
-
-            // Try reading
-            assertRows(cluster.coordinator().execute("SELECT * FROM " + 
KEYSPACE + ".tbl WHERE pk = 1",
-                                                     ConsistencyLevel.ALL),
-                       row(1, 1, 1),
-                       row(1, 2, 2),
-                       row(1, 3, 3));
-
-            // Make sure read-repair did not corrupt the data
-            assertRows(cluster.get(2).executeInternal("SELECT * FROM " + 
KEYSPACE + ".tbl WHERE pk = 1"),
-                       row(1, 1, null),
-                       row(1, 2, null),
-                       row(1, 3, null));
-
-            // Writing to the replica with disagreeing schema should not work
-            Exception thrown = null;
-            try
-            {
-                cluster.coordinator().execute("INSERT INTO " + KEYSPACE + 
".tbl (pk, ck, v1) VALUES (?, ?, ?)",
-                                              ConsistencyLevel.ALL,
-                                              1, 1, 5);
-            }
-            catch (Exception e)
-            {
-                thrown = e;
-            }
-
-            Assert.assertNotNull(thrown);
-
-            thrown = null;
-
-            // If somehow replica got new data, reading that data should not 
be possible, either
-            cluster.get(2).executeInternal("INSERT INTO " + KEYSPACE + ".tbl 
(pk, ck, v1) VALUES (?, ?, ?)",
-                                           1, 1, 100);
-
-            try
-            {
-                assertRows(cluster.coordinator().execute("SELECT * FROM " + 
KEYSPACE + ".tbl WHERE pk = 1",
-                                                         ConsistencyLevel.ALL),
-                           row(1, 1, 1),
-                           row(1, 2, 2),
-                           row(1, 3, 3));
-            }
-            catch (Exception e)
-            {
-                thrown = e;
-            }
-
-            Assert.assertNotNull(thrown);
-        }
-    }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/8aec742a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
----------------------------------------------------------------------
diff --git 
a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java 
b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
index 79db6f2..db83eb4 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
@@ -119,6 +119,21 @@ public class AlterTest extends CQLTester
     }
 
     @Test
+    public void testDropAddWithDifferentKind() throws Throwable
+    {
+        createTable("CREATE TABLE %s (a int, b int, c int, d int static, 
PRIMARY KEY (a, b));");
+
+        execute("ALTER TABLE %s DROP c;");
+        execute("ALTER TABLE %s DROP d;");
+
+        assertInvalidMessage("Cannot re-add previously dropped column 'c' of 
kind STATIC, incompatible with previous kind REGULAR",
+                             "ALTER TABLE %s ADD c int static;");
+
+        assertInvalidMessage("Cannot re-add previously dropped column 'd' of 
kind REGULAR, incompatible with previous kind STATIC",
+                             "ALTER TABLE %s ADD d int;");
+    }
+
+    @Test
     public void testDropStaticWithTimestamp() throws Throwable
     {
         createTable("CREATE TABLE %s (id int, c1 int, v1 int, todrop int 
static, PRIMARY KEY (id, c1));");


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to