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 (" +

Reply via email to