From a5767a58343f7c954e2a5bdb36811f909483dd97 Mon Sep 17 00:00:00 2001 From: Stefan Miklosovic Date: Wed, 3 Jul 2024 13:49:42 +0200 Subject: [PATCH] Fix schema.cql created by a snapshot after dropping more than one column patch by Stefan Miklosovic; reviewed Benjamin Lerer, Francisco Guerrero for CASSANDRA-19747 --- CHANGES.txt | 1 + .../cassandra/schema/TableMetadata.java | 2 +- .../cassandra/db/SchemaCQLHelperTest.java | 84 +++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index 52ec8bc211..d029c330ae 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0.14 + * Fix schema.cql created by a snapshot after dropping more than one column (CASSANDRA-19747) * UnsupportedOperationException when reducing scope for LCS compactions (CASSANDRA-19704) * Make LWT conditions behavior on frozen and non-frozen columns consistent for null column values (CASSANDRA-19637) * Add timeout specifically for bootstrapping nodes (CASSANDRA-15439) diff --git a/src/java/org/apache/cassandra/schema/TableMetadata.java b/src/java/org/apache/cassandra/schema/TableMetadata.java index 36f2382318..bd51349c72 100644 --- a/src/java/org/apache/cassandra/schema/TableMetadata.java +++ b/src/java/org/apache/cassandra/schema/TableMetadata.java @@ -1237,7 +1237,7 @@ public class TableMetadata implements SchemaElement DroppedColumn dropped = iterDropped.next(); dropped.column.appendCqlTo(builder); - if (!hasSingleColumnPrimaryKey || iter.hasNext()) + if (!hasSingleColumnPrimaryKey || iterDropped.hasNext()) builder.append(','); builder.newLine(); diff --git a/test/unit/org/apache/cassandra/db/SchemaCQLHelperTest.java b/test/unit/org/apache/cassandra/db/SchemaCQLHelperTest.java index 8cb1e1508a..ed68b35c26 100644 --- a/test/unit/org/apache/cassandra/db/SchemaCQLHelperTest.java +++ b/test/unit/org/apache/cassandra/db/SchemaCQLHelperTest.java @@ -451,6 +451,90 @@ public class SchemaCQLHelperTest extends CQLTester Assert.assertEquals(2, files.size()); } + @Test + public void testSnapshotWithDroppedColumnsWithoutReAdding() throws Throwable + { + String tableName = createTable("CREATE TABLE IF NOT EXISTS %s (" + + "pk1 varint," + + "pk2 ascii," + + "ck1 varint," + + "ck2 varint," + + "reg1 int," + + "reg2 int," + + "reg3 int," + + "PRIMARY KEY ((pk1, pk2), ck1, ck2)) WITH " + + "CLUSTERING ORDER BY (ck1 ASC, ck2 DESC);"); + + alterTable("ALTER TABLE %s DROP reg2 USING TIMESTAMP 10000;"); + alterTable("ALTER TABLE %s DROP reg3 USING TIMESTAMP 10000;"); + + for (int i = 0; i < 10; i++) + execute("INSERT INTO %s (pk1, pk2, ck1, ck2, reg1) VALUES (?, ?, ?, ?, ?)", i, i + 1, i + 2, i + 3, null); + + ColumnFamilyStore cfs = Keyspace.open(keyspace()).getColumnFamilyStore(tableName); + cfs.snapshot(SNAPSHOT); + + String schema = Files.toString(cfs.getDirectories().getSnapshotSchemaFile(SNAPSHOT), Charset.defaultCharset()); + schema = schema.substring(schema.indexOf("CREATE TABLE")); // trim to ensure order + String expected = "CREATE TABLE IF NOT EXISTS " + keyspace() + "." + tableName + " (\n" + + " pk1 varint,\n" + + " pk2 ascii,\n" + + " ck1 varint,\n" + + " ck2 varint,\n" + + " reg1 int,\n" + + " reg3 int,\n" + + " reg2 int,\n" + + " PRIMARY KEY ((pk1, pk2), ck1, ck2)\n" + + ") WITH ID = " + cfs.metadata.id + "\n" + + " AND CLUSTERING ORDER BY (ck1 ASC, ck2 DESC)"; + + assertThat(schema, + allOf(startsWith(expected), + containsString("ALTER TABLE " + keyspace() + "." + tableName + " DROP reg2 USING TIMESTAMP 10000;"), + containsString("ALTER TABLE " + keyspace() + "." + tableName + " DROP reg3 USING TIMESTAMP 10000;"))); + + JSONObject manifest = (JSONObject) new JSONParser().parse(new FileReader(cfs.getDirectories().getSnapshotManifestFile(SNAPSHOT))); + JSONArray files = (JSONArray) manifest.get("files"); + Assert.assertEquals(1, files.size()); + } + + @Test + public void testSnapshotWithDroppedColumnsWithoutReAddingOnSingleKeyTable() throws Throwable + { + String tableName = createTable("CREATE TABLE IF NOT EXISTS %s (" + + "pk1 varint PRIMARY KEY," + + "reg1 int," + + "reg2 int," + + "reg3 int);"); + + alterTable("ALTER TABLE %s DROP reg2 USING TIMESTAMP 10000;"); + alterTable("ALTER TABLE %s DROP reg3 USING TIMESTAMP 10000;"); + + for (int i = 0; i < 10; i++) + execute("INSERT INTO %s (pk1, reg1) VALUES (?, ?)", i, i + 1); + + ColumnFamilyStore cfs = Keyspace.open(keyspace()).getColumnFamilyStore(tableName); + cfs.snapshot(SNAPSHOT); + + String schema = Files.toString(cfs.getDirectories().getSnapshotSchemaFile(SNAPSHOT), Charset.defaultCharset()); + schema = schema.substring(schema.indexOf("CREATE TABLE")); // trim to ensure order + String expected = "CREATE TABLE IF NOT EXISTS " + keyspace() + "." + tableName + " (\n" + + " pk1 varint PRIMARY KEY,\n" + + " reg1 int,\n" + + " reg3 int,\n" + + " reg2 int\n" + + ") WITH ID = " + cfs.metadata.id + "\n"; + + assertThat(schema, + allOf(startsWith(expected), + containsString("ALTER TABLE " + keyspace() + "." + tableName + " DROP reg2 USING TIMESTAMP 10000;"), + containsString("ALTER TABLE " + keyspace() + "." + tableName + " DROP reg3 USING TIMESTAMP 10000;"))); + + JSONObject manifest = (JSONObject) new JSONParser().parse(new FileReader(cfs.getDirectories().getSnapshotManifestFile(SNAPSHOT))); + JSONArray files = (JSONArray) manifest.get("files"); + Assert.assertEquals(1, files.size()); + } + @Test public void testSystemKsSnapshot() {