diff --git a/src/java/org/apache/cassandra/db/SerializationHeader.java b/src/java/org/apache/cassandra/db/SerializationHeader.java index 11239d8bc3..20d52276a3 100644 --- a/src/java/org/apache/cassandra/db/SerializationHeader.java +++ b/src/java/org/apache/cassandra/db/SerializationHeader.java @@ -88,13 +88,30 @@ public class SerializationHeader // but rather on their stats stored in StatsMetadata that are fully accurate. EncodingStats.Collector stats = new EncodingStats.Collector(); RegularAndStaticColumns.Builder columns = RegularAndStaticColumns.builder(); + RegularAndStaticColumns currentSchemaColumns = metadata.regularAndStaticColumns(); // We need to order the SSTables by descending generation to be sure that we use latest column metadata. for (SSTableReader sstable : orderByDescendingGeneration(sstables)) { stats.updateTimestamp(sstable.getMinTimestamp()); stats.updateLocalDeletionTime(sstable.getMinLocalDeletionTime()); stats.updateTTL(sstable.getMinTTL()); - columns.addAll(sstable.header.columns()); + for (ColumnMetadata column : sstable.header.columns()) + { + // Include column in a header only if it is in the current schema. + // It is not enough to check if a column is not among dropped ones, + // because that would not cover the scenario when a column is dropped + // and re-added back before flush. Consider: + // + // insert into ks.tb (id, val) values (1, 2); + // alter table ks.tb drop val; + // alter table ks.tb add val int; + // + // If we backed our logic only by a check against a dropped column, + // then we would see that it was dropped (in metadata.droppedColumn) + // but we would omit the fact that it was actually re-added. + if (currentSchemaColumns.contains(column)) + columns.add(column); + } } return new SerializationHeader(true, metadata, columns.build(), stats.get()); } diff --git a/test/unit/org/apache/cassandra/db/SerializationHeaderCompactionCQLTest.java b/test/unit/org/apache/cassandra/db/SerializationHeaderCompactionCQLTest.java new file mode 100644 index 0000000000..8ea0ae640c --- /dev/null +++ b/test/unit/org/apache/cassandra/db/SerializationHeaderCompactionCQLTest.java @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db; + +import org.junit.Test; + +import org.apache.cassandra.cql3.CQLTester; +import org.apache.cassandra.io.sstable.format.SSTableReader; +import org.apache.cassandra.schema.ColumnMetadata; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class SerializationHeaderCompactionCQLTest extends CQLTester +{ + @Test + public void testDroppedColumnPresenceInSerialisationHeader() throws Throwable + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, a text, b text, c text)"); + execute("INSERT INTO %s (id, a, b, c) VALUES (1, 'a1', 'b1', 'c1')"); + flush(keyspace()); + alterTable("ALTER TABLE %s DROP b"); + + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + assertEquals(1, cfs.getLiveSSTables().size()); + + SSTableReader sstable = cfs.getLiveSSTables().iterator().next(); + assertTrue("Column 'b' should be in the flushed SSTable header", + sstableHeaderContainsColumn(sstable, "b")); + + // same but alter before flush + + createTable("CREATE TABLE %s (id int PRIMARY KEY, a text, b text, c text)"); + execute("INSERT INTO %s (id, a, b, c) VALUES (1, 'a1', 'b1', 'c1')"); + alterTable("ALTER TABLE %s DROP b"); + flush(keyspace()); + + ColumnFamilyStore cfs2 = getCurrentColumnFamilyStore(); + assertEquals(1, cfs2.getLiveSSTables().size()); + + SSTableReader sstable2 = cfs2.getLiveSSTables().iterator().next(); + assertTrue("Column 'b' should be in the flushed SSTable header", + sstableHeaderContainsColumn(sstable2, "b")); + + // flush but without dropped column + + createTable("CREATE TABLE %s (id int PRIMARY KEY, a text, b text, c text)"); + // we are not populating column we go to drop on purpose + execute("INSERT INTO %s (id, a, c) VALUES (1, 'a1', 'c1')"); + alterTable("ALTER TABLE %s DROP b"); + flush(keyspace()); + + ColumnFamilyStore cfs3 = getCurrentColumnFamilyStore(); + assertEquals(1, cfs3.getLiveSSTables().size()); + + SSTableReader sstable3 = cfs3.getLiveSSTables().iterator().next(); + assertFalse("Column 'b' should not be in the flushed SSTable header", + sstableHeaderContainsColumn(sstable3, "b")); + } + + @Test + public void testDroppedColumnNotInCompactedSerialisationHeader() throws Throwable + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, a text, b text, c text)"); + execute("INSERT INTO %s (id, a, b, c) VALUES (1, 'a1', 'b1', 'c1')"); + alterTable("ALTER TABLE %s DROP b"); + flush(keyspace()); + + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + assertEquals(1, cfs.getLiveSSTables().size()); + assertTrue("Column 'b' should be in the flushed SSTable header", + sstableHeaderContainsColumn(cfs.getLiveSSTables().iterator().next(), "b")); + + compact(); + + assertEquals(1, cfs.getLiveSSTables().size()); + + // column dropped for a header after compaction + assertFalse("Dropped column 'b' should not be in the compacted SSTable header", + sstableHeaderContainsColumn(cfs.getLiveSSTables().iterator().next(), "b")); + + assertTrue("Column 'a' should be in the compacted SSTable header", + sstableHeaderContainsColumn(cfs.getLiveSSTables().iterator().next(), "a")); + assertTrue("Column 'c' should be in the compacted SSTable header", + sstableHeaderContainsColumn(cfs.getLiveSSTables().iterator().next(), "c")); + + assertRows(execute("SELECT id, a, c FROM %s"), row(1, "a1", "c1")); + } + + @Test + public void testReAddedColumnInCompactedSerializationHeader() throws Throwable + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, a text, b text, c text)"); + execute("INSERT INTO %s (id, a, b, c) VALUES (1, 'a1', 'b1', 'c1')"); + alterTable("ALTER TABLE %s DROP b"); + alterTable("ALTER TABLE %s ADD b text"); + flush(keyspace()); + + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + assertEquals(1, cfs.getLiveSSTables().size()); + assertTrue("Column 'b' should be in the flushed SSTable header", + sstableHeaderContainsColumn(cfs.getLiveSSTables().iterator().next(), "b")); + + compact(); + + assertEquals(1, cfs.getLiveSSTables().size()); + + // re-added column b + assertTrue("Dropped column 'b' should be in the compacted SSTable header", + sstableHeaderContainsColumn(cfs.getLiveSSTables().iterator().next(), "b")); + + assertTrue("Column 'a' should be in the compacted SSTable header", + sstableHeaderContainsColumn(cfs.getLiveSSTables().iterator().next(), "a")); + assertTrue("Column 'c' should be in the compacted SSTable header", + sstableHeaderContainsColumn(cfs.getLiveSSTables().iterator().next(), "c")); + + assertRows(execute("SELECT id, a, b, c FROM %s"), row(1, "a1", null, "c1")); + } + + /** + * Checks if a column with the given name exists in the SSTable header. + */ + private boolean sstableHeaderContainsColumn(SSTableReader sstable, String columnName) + { + for (ColumnMetadata column : sstable.header.columns()) + { + if (column.name.toString().equals(columnName)) + return true; + } + return false; + } +}