Return-Path: X-Original-To: apmail-cassandra-commits-archive@www.apache.org Delivered-To: apmail-cassandra-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 1162117313 for ; Fri, 16 Oct 2015 12:58:36 +0000 (UTC) Received: (qmail 95118 invoked by uid 500); 16 Oct 2015 12:58:35 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 95089 invoked by uid 500); 16 Oct 2015 12:58:35 -0000 Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cassandra.apache.org Delivered-To: mailing list commits@cassandra.apache.org Received: (qmail 95029 invoked by uid 99); 16 Oct 2015 12:58:35 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 16 Oct 2015 12:58:35 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 869A5DFBD2; Fri, 16 Oct 2015 12:58:35 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: slebresne@apache.org To: commits@cassandra.apache.org Date: Fri, 16 Oct 2015 12:58:35 -0000 Message-Id: <4ffa906b46074ba4b4e7f9b0e54e7bdb@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [1/2] cassandra git commit: Followup to CASSANDRA-10468: fix reverse case and add unit test Repository: cassandra Updated Branches: refs/heads/trunk 737a3385c -> 4034dc904 Followup to CASSANDRA-10468: fix reverse case and add unit test patch by slebresne; reviewed by blerer for CASSANDRA-10468 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/d413ddff Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/d413ddff Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/d413ddff Branch: refs/heads/trunk Commit: d413ddff8b0bdaef600a31d81da227be99072574 Parents: 1b93eb4 Author: Sylvain Lebresne Authored: Thu Oct 15 12:15:52 2015 +0200 Committer: Sylvain Lebresne Committed: Fri Oct 16 14:57:32 2015 +0200 ---------------------------------------------------------------------- .../org/apache/cassandra/db/rows/BTreeRow.java | 7 +- .../org/apache/cassandra/db/rows/RowsTest.java | 131 ++++++++++++++++++- 2 files changed, 133 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/d413ddff/src/java/org/apache/cassandra/db/rows/BTreeRow.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/rows/BTreeRow.java b/src/java/org/apache/cassandra/db/rows/BTreeRow.java index 0d8eda8..4bd11da 100644 --- a/src/java/org/apache/cassandra/db/rows/BTreeRow.java +++ b/src/java/org/apache/cassandra/db/rows/BTreeRow.java @@ -428,7 +428,7 @@ public class BTreeRow extends AbstractRow private class CellInLegacyOrderIterator extends AbstractIterator { - private final AbstractType comparator; + private final Comparator comparator; private final boolean reversed; private final int firstComplexIdx; private int simpleIdx; @@ -438,7 +438,8 @@ public class BTreeRow extends AbstractRow private CellInLegacyOrderIterator(CFMetaData metadata, boolean reversed) { - this.comparator = metadata.getColumnDefinitionNameComparator(isStatic() ? ColumnDefinition.Kind.STATIC : ColumnDefinition.Kind.REGULAR); + AbstractType nameComparator = metadata.getColumnDefinitionNameComparator(isStatic() ? ColumnDefinition.Kind.STATIC : ColumnDefinition.Kind.REGULAR); + this.comparator = reversed ? Collections.reverseOrder(nameComparator) : nameComparator; this.reversed = reversed; // copy btree into array for simple separate iteration of simple and complex columns @@ -464,7 +465,7 @@ public class BTreeRow extends AbstractRow private int getComplexIdx() { - return reversed ? data.length - complexIdx - 1 : complexIdx; + return reversed ? data.length + firstComplexIdx - complexIdx - 1 : complexIdx; } private int getComplexIdxAndIncrement() http://git-wip-us.apache.org/repos/asf/cassandra/blob/d413ddff/test/unit/org/apache/cassandra/db/rows/RowsTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/rows/RowsTest.java b/test/unit/org/apache/cassandra/db/rows/RowsTest.java index 7ee2d0a..dede867 100644 --- a/test/unit/org/apache/cassandra/db/rows/RowsTest.java +++ b/test/unit/org/apache/cassandra/db/rows/RowsTest.java @@ -39,8 +39,7 @@ import org.apache.cassandra.cql3.ColumnIdentifier; import org.apache.cassandra.db.Clustering; import org.apache.cassandra.db.DeletionTime; import org.apache.cassandra.db.LivenessInfo; -import org.apache.cassandra.db.marshal.IntegerType; -import org.apache.cassandra.db.marshal.MapType; +import org.apache.cassandra.db.marshal.*; import org.apache.cassandra.db.partitions.PartitionStatisticsCollector; import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.FBUtilities; @@ -545,4 +544,132 @@ public class RowsTest Assert.assertEquals(Collections.emptyList(), builder.complexDeletions); Assert.assertEquals(Collections.emptyList(), builder.cells); } + + // Creates a dummy cell for a (regular) column for the provided name and without a cellPath. + private static Cell liveCell(ColumnDefinition name) + { + return liveCell(name, -1); + } + + // Creates a dummy cell for a (regular) column for the provided name. + // If path >= 0, the cell will have a CellPath containing path as an Int32Type. + private static Cell liveCell(ColumnDefinition name, int path) + { + CellPath cp = path < 0 ? null : CellPath.create(ByteBufferUtil.bytes(path)); + return new BufferCell(name, 0L, Cell.NO_TTL, Cell.NO_DELETION_TIME, ByteBuffer.allocate(1), cp); + } + + // Assert that the cells generated by iterating iterable are the cell of cells (in the same order + // and with neither more nor less cells). + private static void assertCellOrder(Iterable iterable, Cell... cells) + { + int i = 0; + for (Cell actual : iterable) + { + Assert.assertFalse(String.format("Got more rows than expected (expecting %d). First unexpected cell is %s", cells.length, actual), i >= cells.length); + Assert.assertEquals(cells[i++], actual); + } + Assert.assertFalse(String.format("Got less rows than expected (got %d while expecting %d).", i, cells.length), i < cells.length); + } + + // Make a dummy row (empty clustering) with the provided cells, that are assumed to be in order + private static Row makeDummyRow(Cell ... cells) + { + Row.Builder builder = BTreeRow.sortedBuilder(); + builder.newRow(Clustering.EMPTY); + for (Cell cell : cells) + builder.addCell(cell); + + return builder.build(); + } + + @Test + public void testLegacyCellIterator() + { + // Creates a table with + // - 3 Simple columns: a, c and e + // - 2 Complex columns: b and d + CFMetaData metadata = CFMetaData.Builder.create("dummy_ks", "dummy_tbl") + .addPartitionKey("k", BytesType.instance) + .addRegularColumn("a", BytesType.instance) + .addRegularColumn("b", MapType.getInstance(Int32Type.instance, BytesType.instance, true)) + .addRegularColumn("c", BytesType.instance) + .addRegularColumn("d", MapType.getInstance(Int32Type.instance, BytesType.instance, true)) + .addRegularColumn("e", BytesType.instance) + .build(); + + ColumnDefinition a = metadata.getColumnDefinition(new ColumnIdentifier("a", false)); + ColumnDefinition b = metadata.getColumnDefinition(new ColumnIdentifier("b", false)); + ColumnDefinition c = metadata.getColumnDefinition(new ColumnIdentifier("c", false)); + ColumnDefinition d = metadata.getColumnDefinition(new ColumnIdentifier("d", false)); + ColumnDefinition e = metadata.getColumnDefinition(new ColumnIdentifier("e", false)); + + Row row; + + // Row with only simple columns + + row = makeDummyRow(liveCell(a), + liveCell(c), + liveCell(e)); + + + assertCellOrder(row.cellsInLegacyOrder(metadata, false), + liveCell(a), + liveCell(c), + liveCell(e)); + + assertCellOrder(row.cellsInLegacyOrder(metadata, true), + liveCell(e), + liveCell(c), + liveCell(a)); + + // Row with only complex columns + + row = makeDummyRow(liveCell(b, 1), + liveCell(b, 2), + liveCell(d, 3), + liveCell(d, 4)); + + + assertCellOrder(row.cellsInLegacyOrder(metadata, false), + liveCell(b, 1), + liveCell(b, 2), + liveCell(d, 3), + liveCell(d, 4)); + + assertCellOrder(row.cellsInLegacyOrder(metadata, true), + liveCell(d, 4), + liveCell(d, 3), + liveCell(b, 2), + liveCell(b, 1)); + + // Row with mixed simple and complex columns + + row = makeDummyRow(liveCell(a), + liveCell(c), + liveCell(e), + liveCell(b, 1), + liveCell(b, 2), + liveCell(d, 3), + liveCell(d, 4)); + + + assertCellOrder(row.cellsInLegacyOrder(metadata, false), + liveCell(a), + liveCell(b, 1), + liveCell(b, 2), + liveCell(c), + liveCell(d, 3), + liveCell(d, 4), + liveCell(e)); + + assertCellOrder(row.cellsInLegacyOrder(metadata, true), + liveCell(e), + liveCell(d, 4), + liveCell(d, 3), + liveCell(c), + liveCell(b, 2), + liveCell(b, 1), + liveCell(a)); + } }