Repository: cassandra Updated Branches: refs/heads/cassandra-3.0 fb606dd41 -> 7c2437e26 refs/heads/cassandra-3.11 3acdcaf8d -> 6b7302b65 refs/heads/trunk bd14400a9 -> a9207a44a
Duplicate rows after upgrading from 2.1.16 to 3.0.10/3.9 patch by Sylvain Lebresne; reviewed by Tyler Hobbs for CASSANDRA-13125 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/7c2437e2 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/7c2437e2 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/7c2437e2 Branch: refs/heads/cassandra-3.0 Commit: 7c2437e26bc289c0a4c40fca303310fb61f362b1 Parents: fb606dd Author: Sylvain Lebresne <[email protected]> Authored: Fri Jan 20 15:14:46 2017 +0100 Committer: Sylvain Lebresne <[email protected]> Committed: Wed Feb 8 11:36:24 2017 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../org/apache/cassandra/db/LegacyLayout.java | 62 ++++++++++++++--- .../apache/cassandra/db/LegacyLayoutTest.java | 71 ++++++++++++++++++++ 3 files changed, 123 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/7c2437e2/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 4387019..7da61e7 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.11 + * Duplicate rows after upgrading from 2.1.16 to 3.0.10/3.9 (CASSANDRA-13125) * Fix UPDATE queries with empty IN restrictions (CASSANDRA-13152) * Abort or retry on failed hints delivery (CASSANDRA-13124) * Fix handling of partition with partition-level deletion plus http://git-wip-us.apache.org/repos/asf/cassandra/blob/7c2437e2/src/java/org/apache/cassandra/db/LegacyLayout.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/LegacyLayout.java b/src/java/org/apache/cassandra/db/LegacyLayout.java index 3f69671..3788c3c 100644 --- a/src/java/org/apache/cassandra/db/LegacyLayout.java +++ b/src/java/org/apache/cassandra/db/LegacyLayout.java @@ -1803,17 +1803,42 @@ public abstract class LegacyLayout if (a.isStatic != b.isStatic) return a.isStatic ? -1 : 1; - int result = this.clusteringComparator.compare(a.bound, b.bound); - if (result != 0) - return result; + // We have to be careful with bound comparison because of collections. Namely, if the 2 bounds represent the + // same prefix, then we should take the collectionName into account before taking the bounds kind + // (ClusteringPrefix.Kind). This means we can't really call ClusteringComparator.compare() directly. + // For instance, if + // a is (bound=INCL_START_BOUND('x'), collectionName='d') + // b is (bound=INCL_END_BOUND('x'), collectionName='c') + // Ten b < a since the element 'c' of collection 'x' comes before element 'd', but calling + // clusteringComparator.compare(a.bound, b.bound) returns -1. + // See CASSANDRA-13125 for details. + int sa = a.bound.size(); + int sb = b.bound.size(); + for (int i = 0; i < Math.min(sa, sb); i++) + { + int cmp = clusteringComparator.compareComponent(i, a.bound.get(i), b.bound.get(i)); + if (cmp != 0) + return cmp; + } + + if (sa != sb) + return sa < sb ? a.bound.kind().comparedToClustering : -b.bound.kind().comparedToClustering; + + // Both bound represent the same prefix, compare the collection names + // If one has a collection name and the other doesn't, the other comes before as it points to the beginning of the row. + if ((a.collectionName == null) != (b.collectionName == null)) + return a.collectionName == null ? -1 : 1; - // If both have equal "bound" but one is a collection tombstone and not the other, then the other comes before as it points to the beginning of the row. - if (a.collectionName == null) - return b.collectionName == null ? 0 : 1; - if (b.collectionName == null) - return -1; + // If they both have a collection, compare that first + if (a.collectionName != null) + { + int cmp = UTF8Type.instance.compare(a.collectionName.name.bytes, b.collectionName.name.bytes); + if (cmp != 0) + return cmp; + } - return UTF8Type.instance.compare(a.collectionName.name.bytes, b.collectionName.name.bytes); + // Lastly, if everything so far is equal, compare their clustering kind + return ClusteringPrefix.Kind.compare(a.bound.kind(), b.bound.kind()); } } @@ -1830,8 +1855,8 @@ public abstract class LegacyLayout // Note: we don't want to use a List for the markedAts and delTimes to avoid boxing. We could // use a List for starts and ends, but having arrays everywhere is almost simpler. - private LegacyBound[] starts; - private LegacyBound[] ends; + LegacyBound[] starts; + LegacyBound[] ends; private long[] markedAts; private int[] delTimes; @@ -1853,6 +1878,20 @@ public abstract class LegacyLayout this(comparator, new LegacyBound[capacity], new LegacyBound[capacity], new long[capacity], new int[capacity], 0); } + @Override + public String toString() + { + StringBuilder sb = new StringBuilder(); + sb.append('['); + for (int i = 0; i < size; i++) + { + if (i > 0) + sb.append(','); + sb.append('(').append(starts[i]).append(", ").append(ends[i]).append(')'); + } + return sb.append(']').toString(); + } + public boolean isEmpty() { return size == 0; @@ -2107,6 +2146,7 @@ public abstract class LegacyLayout // If we got there, then just insert the remainder at the end addInternal(i, start, end, markedAt, delTime); } + private int capacity() { return starts.length; http://git-wip-us.apache.org/repos/asf/cassandra/blob/7c2437e2/test/unit/org/apache/cassandra/db/LegacyLayoutTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/LegacyLayoutTest.java b/test/unit/org/apache/cassandra/db/LegacyLayoutTest.java new file mode 100644 index 0000000..09e9755 --- /dev/null +++ b/test/unit/org/apache/cassandra/db/LegacyLayoutTest.java @@ -0,0 +1,71 @@ +/* + * 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 java.nio.ByteBuffer; + +import org.junit.Test; + +import org.apache.cassandra.config.CFMetaData; +import org.apache.cassandra.config.ColumnDefinition; +import org.apache.cassandra.cql3.ColumnIdentifier; +import org.apache.cassandra.db.marshal.Int32Type; +import org.apache.cassandra.db.marshal.SetType; +import org.apache.cassandra.db.partitions.PartitionUpdate; +import org.apache.cassandra.db.rows.BTreeRow; +import org.apache.cassandra.db.rows.Row; +import org.apache.cassandra.utils.ByteBufferUtil; + +import static org.junit.Assert.*; + +public class LegacyLayoutTest +{ + @Test + public void testFromUnfilteredRowIterator() throws Throwable + { + CFMetaData table = CFMetaData.Builder.create("ks", "table") + .addPartitionKey("k", Int32Type.instance) + .addRegularColumn("a", SetType.getInstance(Int32Type.instance, true)) + .addRegularColumn("b", SetType.getInstance(Int32Type.instance, true)) + .build(); + + ColumnDefinition a = table.getColumnDefinition(new ColumnIdentifier("a", false)); + ColumnDefinition b = table.getColumnDefinition(new ColumnIdentifier("b", false)); + + Row.Builder builder = BTreeRow.unsortedBuilder(0); + builder.newRow(Clustering.EMPTY); + builder.addComplexDeletion(a, new DeletionTime(1L, 1)); + builder.addComplexDeletion(b, new DeletionTime(1L, 1)); + Row row = builder.build(); + + ByteBuffer key = ByteBufferUtil.bytes(1); + PartitionUpdate upd = PartitionUpdate.singleRowUpdate(table, key, row); + + LegacyLayout.LegacyUnfilteredPartition p = LegacyLayout.fromUnfilteredRowIterator(null, upd.unfilteredIterator()); + assertEquals(DeletionTime.LIVE, p.partitionDeletion); + assertEquals(0, p.cells.size()); + + LegacyLayout.LegacyRangeTombstoneList l = p.rangeTombstones; + assertEquals("a", l.starts[0].collectionName.name.toString()); + assertEquals("a", l.ends[0].collectionName.name.toString()); + + assertEquals("b", l.starts[1].collectionName.name.toString()); + assertEquals("b", l.ends[1].collectionName.name.toString()); + } +} \ No newline at end of file
