You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by ca...@apache.org on 2016/01/07 16:40:36 UTC

[2/3] cassandra git commit: Fix bad merge

Fix bad merge


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/caedc8fd
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/caedc8fd
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/caedc8fd

Branch: refs/heads/trunk
Commit: caedc8fdcf1a449531e03c96b82db629809e10ab
Parents: f7843d2
Author: Carl Yeksigian <ca...@apache.org>
Authored: Thu Jan 7 10:37:21 2016 -0500
Committer: Carl Yeksigian <ca...@apache.org>
Committed: Thu Jan 7 10:37:21 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/caedc8fd/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/caedc8fd/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 (" +