Fix value skipping with counter columns patch by Sylvain Lebresne; reviewed by Aleksey Yeschenko for CASSANDRA-11726
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/ee609411 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/ee609411 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/ee609411 Branch: refs/heads/cassandra-3.8 Commit: ee609411a8e154255812b157788a79bbdf076566 Parents: 02ebf87 Author: Sylvain Lebresne <sylv...@datastax.com> Authored: Thu Aug 4 17:54:03 2016 +0200 Committer: Aleksey Yeschenko <alek...@apache.org> Committed: Tue Aug 9 16:33:10 2016 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + src/java/org/apache/cassandra/db/Conflicts.java | 9 +++++++ .../cql3/validation/entities/CountersTest.java | 27 ++++++++++++++++++++ 3 files changed, 37 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/ee609411/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 2c5c221..b7bbf72 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.9 + * Fix value skipping with counter columns (CASSANDRA-11726) * Fix nodetool tablestats miss SSTable count (CASSANDRA-12205) * Fixed flacky SSTablesIteratedTest (CASSANDRA-12282) * Fixed flacky SSTableRewriterTest: check file counts before calling validateCFS (CASSANDRA-12348) http://git-wip-us.apache.org/repos/asf/cassandra/blob/ee609411/src/java/org/apache/cassandra/db/Conflicts.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/Conflicts.java b/src/java/org/apache/cassandra/db/Conflicts.java index fa0e819..9e8bd9a 100644 --- a/src/java/org/apache/cassandra/db/Conflicts.java +++ b/src/java/org/apache/cassandra/db/Conflicts.java @@ -68,6 +68,15 @@ public abstract class Conflicts if (!rightLive) return Resolution.RIGHT_WINS; + // Handle empty values. Counters can't truly have empty values, but we can have a counter cell that temporarily + // has one on read if the column for the cell is not queried by the user due to the optimization of #10657. We + // thus need to handle this (see #11726 too). + if (!leftValue.hasRemaining()) + return rightValue.hasRemaining() || leftTimestamp > rightTimestamp ? Resolution.LEFT_WINS : Resolution.RIGHT_WINS; + + if (!rightValue.hasRemaining()) + return Resolution.RIGHT_WINS; + return Resolution.MERGE; } http://git-wip-us.apache.org/repos/asf/cassandra/blob/ee609411/test/unit/org/apache/cassandra/cql3/validation/entities/CountersTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/CountersTest.java b/test/unit/org/apache/cassandra/cql3/validation/entities/CountersTest.java index 33b4a4f..b1cd0bb 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/entities/CountersTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/entities/CountersTest.java @@ -196,4 +196,31 @@ public class CountersTest extends CQLTester assertInvalidThrowMessage("counter type is not supported for PRIMARY KEY part a", InvalidRequestException.class, String.format("CREATE TABLE %s.%s (a counter, b int, PRIMARY KEY (b, a)) WITH CLUSTERING ORDER BY (a desc);", KEYSPACE, createTableName())); } + + /** + * Test for the bug of #11726. + */ + @Test + public void testCounterAndColumnSelection() throws Throwable + { + for (String compactStorageClause : new String[]{ "", " WITH COMPACT STORAGE" }) + { + createTable("CREATE TABLE %s (k int PRIMARY KEY, c counter)" + compactStorageClause); + + // Flush 2 updates in different sstable so that the following select does a merge, which is what triggers + // the problem from #11726 + + execute("UPDATE %s SET c = c + ? WHERE k = ?", 1L, 0); + + flush(); + + execute("UPDATE %s SET c = c + ? WHERE k = ?", 1L, 0); + + flush(); + + // Querying, but not including the counter. Pre-CASSANDRA-11726, this made us query the counter but include + // it's value, which broke at merge (post-CASSANDRA-11726 are special cases to never skip values). + assertRows(execute("SELECT k FROM %s"), row(0)); + } + } }
