Repository: cassandra Updated Branches: refs/heads/cassandra-3.0 f937c8bee -> 70c08ece5 refs/heads/cassandra-3.3 7fbc39b74 -> 91aeb2637 refs/heads/trunk 786d67675 -> fa1707fa6
MV timestamp should be the maximum of the values, not the minimum patch by Carl Yeksigian; reviewed by Jake Luciani for CASSANDRA-10910 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/70c08ece Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/70c08ece Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/70c08ece Branch: refs/heads/cassandra-3.0 Commit: 70c08ece563731cd546d24541f225c888f4d02f5 Parents: f937c8b Author: Carl Yeksigian <c...@apache.org> Authored: Wed Jan 6 10:41:47 2016 -0500 Committer: Carl Yeksigian <c...@apache.org> Committed: Wed Jan 6 10:42:36 2016 -0500 ---------------------------------------------------------------------- .../apache/cassandra/db/view/TemporalRow.java | 65 ++++++++++++++------ .../org/apache/cassandra/cql3/ViewTest.java | 30 +++++++++ 2 files changed, 76 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/70c08ece/src/java/org/apache/cassandra/db/view/TemporalRow.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/view/TemporalRow.java b/src/java/org/apache/cassandra/db/view/TemporalRow.java index 8898857..8ee310d 100644 --- a/src/java/org/apache/cassandra/db/view/TemporalRow.java +++ b/src/java/org/apache/cassandra/db/view/TemporalRow.java @@ -22,6 +22,7 @@ import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -279,9 +280,7 @@ public class TemporalRow this.nowInSec = nowInSec; LivenessInfo liveness = row.primaryKeyLivenessInfo(); - this.viewClusteringLocalDeletionTime = minValueIfSet(viewClusteringLocalDeletionTime, row.deletion().time().localDeletionTime(), NO_DELETION_TIME); - this.viewClusteringTimestamp = minValueIfSet(viewClusteringTimestamp, liveness.timestamp(), NO_TIMESTAMP); - this.viewClusteringTtl = minValueIfSet(viewClusteringTtl, liveness.ttl(), NO_TTL); + updateLiveness(liveness.ttl(), liveness.timestamp(), row.deletion().time().localDeletionTime()); List<ColumnDefinition> clusteringDefs = baseCfs.metadata.clusteringColumns(); clusteringColumns = new HashMap<>(); @@ -295,6 +294,31 @@ public class TemporalRow } } + /* + * PK ts:5, ttl:1, deletion: 2 + * Col ts:4, ttl:2, deletion: 3 + * + * TTL use min, since it expires at the lowest time which we are expiring. If we have the above values, we + * would want to return 1, since the base row expires in 1 second. + * + * Timestamp uses max, as this is the time that the row has been written to the view. See CASSANDRA-10910. + * + * Local Deletion Time should use max, as this deletion will cover all previous values written. + */ + @SuppressWarnings("unchecked") + private void updateLiveness(int ttl, long timestamp, int localDeletionTime) + { + // We are returning whichever is higher from valueIfSet + // Natural order will return the max: 1.compareTo(2) < 0, so 2 is returned + // Reverse order will return the min: 1.compareTo(2) > 0, so 1 is returned + final Comparator max = Comparator.naturalOrder(); + final Comparator min = Comparator.reverseOrder(); + + this.viewClusteringTtl = valueIfSet(viewClusteringTtl, ttl, NO_TTL, min); + this.viewClusteringTimestamp = valueIfSet(viewClusteringTimestamp, timestamp, NO_TIMESTAMP, max); + this.viewClusteringLocalDeletionTime = valueIfSet(viewClusteringLocalDeletionTime, localDeletionTime, NO_DELETION_TIME, max); + } + @Override public String toString() { @@ -351,30 +375,33 @@ public class TemporalRow // If this column is part of the view's primary keys if (viewPrimaryKey.contains(identifier)) { - this.viewClusteringTtl = minValueIfSet(this.viewClusteringTtl, ttl, NO_TTL); - this.viewClusteringTimestamp = minValueIfSet(this.viewClusteringTimestamp, timestamp, NO_TIMESTAMP); - this.viewClusteringLocalDeletionTime = minValueIfSet(this.viewClusteringLocalDeletionTime, localDeletionTime, NO_DELETION_TIME); + updateLiveness(ttl, timestamp, localDeletionTime); } innerMap.get(cellPath).setVersion(new TemporalCell(value, timestamp, ttl, localDeletionTime, isNew)); } - private static int minValueIfSet(int existing, int update, int defaultValue) - { - if (existing == defaultValue) - return update; - if (update == defaultValue) - return existing; - return Math.min(existing, update); - } - - private static long minValueIfSet(long existing, long update, long defaultValue) + /** + * @return + * <ul> + * <li> + * If both existing and update are defaultValue, return defaultValue + * </li> + * <li> + * If only one of existing or existing are defaultValue, return the one which is not + * </li> + * <li> + * If both existing and update are not defaultValue, compare using comparator and return the higher one. + * </li> + * </ul> + */ + private static <T> T valueIfSet(T existing, T update, T defaultValue, Comparator<T> comparator) { - if (existing == defaultValue) + if (existing.equals(defaultValue)) return update; - if (update == defaultValue) + if (update.equals(defaultValue)) return existing; - return Math.min(existing, update); + return comparator.compare(existing, update) > 0 ? existing : update; } public int viewClusteringTtl() http://git-wip-us.apache.org/repos/asf/cassandra/blob/70c08ece/test/unit/org/apache/cassandra/cql3/ViewTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/ViewTest.java b/test/unit/org/apache/cassandra/cql3/ViewTest.java index 8ae21df..2e3cf5f 100644 --- a/test/unit/org/apache/cassandra/cql3/ViewTest.java +++ b/test/unit/org/apache/cassandra/cql3/ViewTest.java @@ -275,6 +275,36 @@ public class ViewTest extends CQLTester } @Test + public void testRegularColumnTimestampUpdates() throws Throwable + { + // Regression test for CASSANDRA-10910 + + createTable("CREATE TABLE %s (" + + "k int PRIMARY KEY, " + + "c int, " + + "val int)"); + + execute("USE " + keyspace()); + executeNet(protocolVersion, "USE " + keyspace()); + + createView("mv_rctstest", "CREATE MATERIALIZED VIEW %s AS SELECT * FROM %%s WHERE k IS NOT NULL AND c IS NOT NULL PRIMARY KEY (k,c)"); + + updateView("UPDATE %s SET c = ?, val = ? WHERE k = ?", 0, 0, 0); + updateView("UPDATE %s SET val = ? WHERE k = ?", 1, 0); + updateView("UPDATE %s SET c = ? WHERE k = ?", 1, 0); + assertRows(execute("SELECT c, k, val FROM mv_rctstest"), row(1, 0, 1)); + + updateView("TRUNCATE %s"); + + updateView("UPDATE %s USING TIMESTAMP 1 SET c = ?, val = ? WHERE k = ?", 0, 0, 0); + updateView("UPDATE %s USING TIMESTAMP 3 SET c = ? WHERE k = ?", 1, 0); + updateView("UPDATE %s USING TIMESTAMP 2 SET val = ? WHERE k = ?", 1, 0); + updateView("UPDATE %s USING TIMESTAMP 4 SET c = ? WHERE k = ?", 2, 0); + updateView("UPDATE %s USING TIMESTAMP 3 SET val = ? WHERE k = ?", 2, 0); + assertRows(execute("SELECT c, k, val FROM mv_rctstest"), row(2, 0, 2)); + } + + @Test public void testCountersTable() throws Throwable { createTable("CREATE TABLE %s (" +
