You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by jb...@apache.org on 2011/11/08 18:55:17 UTC
svn commit: r1199367 - in /cassandra/branches/cassandra-1.0: ./
src/java/org/apache/cassandra/db/ test/unit/org/apache/cassandra/db/
Author: jbellis
Date: Tue Nov 8 17:55:16 2011
New Revision: 1199367
URL: http://svn.apache.org/viewvc?rev=1199367&view=rev
Log:
fix querying supercolumns by name
patch by jbellis; reviewed by slebresne and tested by Mike Smith for CASSANDRA-3446
Modified:
cassandra/branches/cassandra-1.0/CHANGES.txt
cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/CollationController.java
cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/Column.java
cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/IColumn.java
cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/SuperColumn.java
cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/TreeMapBackedSortedColumns.java
cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/ColumnFamilyTest.java
cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/TableTest.java
Modified: cassandra/branches/cassandra-1.0/CHANGES.txt
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-1.0/CHANGES.txt?rev=1199367&r1=1199366&r2=1199367&view=diff
==============================================================================
--- cassandra/branches/cassandra-1.0/CHANGES.txt (original)
+++ cassandra/branches/cassandra-1.0/CHANGES.txt Tue Nov 8 17:55:16 2011
@@ -2,6 +2,8 @@
* fix invalidate-related test failures (CASSANDRA-3437)
* add next-gen cqlsh to bin/
* (CQL) fix handling of rows with no columns (CASSANDRA-3424, 3473)
+ * fix querying supercolumns by name returning only a subset of
+ subcolumns or old subcolumn versions (CASSANDRA-3446)
Merged from 0.8:
* Make counter shard merging thread safe (CASSANDRA-3178)
* fix updating CF row_cache_provider (CASSANDRA-3414)
@@ -12,6 +14,7 @@ Merged from 0.8:
* Revert CASSANDRA-2855
* Fix bug preventing the use of efficient cross-DC writes (CASSANDRA-3472)
+
1.0.2
* "defragment" rows for name-based queries under STCS (CASSANDRA-2503)
* cleanup usage of StorageService.setMode() (CASANDRA-3388)
Modified: cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/CollationController.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/CollationController.java?rev=1199367&r1=1199366&r2=1199367&view=diff
==============================================================================
--- cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/CollationController.java (original)
+++ cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/CollationController.java Tue Nov 8 17:55:16 2011
@@ -59,7 +59,9 @@ public class CollationController
public ColumnFamily getTopLevelColumns()
{
- return filter.filter instanceof NamesQueryFilter && cfs.metadata.getDefaultValidator() != CounterColumnType.instance
+ return filter.filter instanceof NamesQueryFilter
+ && (cfs.metadata.cfType == ColumnFamilyType.Standard || filter.path.superColumnName != null)
+ && cfs.metadata.getDefaultValidator() != CounterColumnType.instance
? collectTimeOrderedData()
: collectAllData();
}
@@ -95,8 +97,7 @@ public class CollationController
// avoid changing the filter columns of the original filter
// (reduceNameFilter removes columns that are known to be irrelevant)
- TreeSet<ByteBuffer> filterColumns = new TreeSet<ByteBuffer>(cfs.metadata.comparator);
- filterColumns.addAll(((NamesQueryFilter) filter.filter).columns);
+ TreeSet<ByteBuffer> filterColumns = new TreeSet<ByteBuffer>(((NamesQueryFilter) filter.filter).columns);
QueryFilter reducedFilter = new QueryFilter(filter.key, filter.path, new NamesQueryFilter(filterColumns));
/* add the SSTables on disk */
@@ -181,9 +182,9 @@ public class CollationController
*/
private void reduceNameFilter(QueryFilter filter, ColumnFamily returnCF, long sstableTimestamp)
{
- AbstractColumnContainer container = filter.path.superColumnName != null
- ? (SuperColumn) returnCF.getColumn(filter.path.superColumnName)
- : returnCF;
+ AbstractColumnContainer container = filter.path.superColumnName == null
+ ? returnCF
+ : (SuperColumn) returnCF.getColumn(filter.path.superColumnName);
if (container == null)
return;
@@ -191,7 +192,7 @@ public class CollationController
{
ByteBuffer filterColumn = iterator.next();
IColumn column = container.getColumn(filterColumn);
- if (column != null && column.minTimestamp() > sstableTimestamp)
+ if (column != null && column.timestamp() > sstableTimestamp)
iterator.remove();
}
}
Modified: cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/Column.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/Column.java?rev=1199367&r1=1199366&r2=1199367&view=diff
==============================================================================
--- cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/Column.java (original)
+++ cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/Column.java Tue Nov 8 17:55:16 2011
@@ -104,11 +104,6 @@ public class Column implements IColumn
return timestamp;
}
- public long minTimestamp()
- {
- return timestamp;
- }
-
public boolean isMarkedForDelete()
{
return false;
Modified: cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/IColumn.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/IColumn.java?rev=1199367&r1=1199366&r2=1199367&view=diff
==============================================================================
--- cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/IColumn.java (original)
+++ cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/IColumn.java Tue Nov 8 17:55:16 2011
@@ -81,10 +81,4 @@ public interface IColumn
* For a super column, this is the max column timestamp of the sub columns.
*/
public long maxTimestamp();
-
- /**
- * For a standard column, this is the same as timestamp().
- * For a super column, this is the min column timestamp of the sub columns.
- */
- public long minTimestamp();
}
Modified: cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/SuperColumn.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/SuperColumn.java?rev=1199367&r1=1199366&r2=1199367&view=diff
==============================================================================
--- cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/SuperColumn.java (original)
+++ cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/SuperColumn.java Tue Nov 8 17:55:16 2011
@@ -138,14 +138,6 @@ public class SuperColumn extends Abstrac
return maxTimestamp;
}
- public long minTimestamp()
- {
- long minTimestamp = getMarkedForDeleteAt();
- for (IColumn subColumn : getSubColumns())
- minTimestamp = Math.min(minTimestamp, subColumn.maxTimestamp());
- return minTimestamp;
- }
-
public long mostRecentLiveChangeAt()
{
long max = Long.MIN_VALUE;
Modified: cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/TreeMapBackedSortedColumns.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/TreeMapBackedSortedColumns.java?rev=1199367&r1=1199366&r2=1199367&view=diff
==============================================================================
--- cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/TreeMapBackedSortedColumns.java (original)
+++ cassandra/branches/cassandra-1.0/src/java/org/apache/cassandra/db/TreeMapBackedSortedColumns.java Tue Nov 8 17:55:16 2011
@@ -84,13 +84,21 @@ public class TreeMapBackedSortedColumns
public void addColumn(IColumn column, Allocator allocator)
{
ByteBuffer name = column.name();
+ // this is a slightly unusual way to structure this; a more natural way is shown in ThreadSafeSortedColumns,
+ // but TreeMap lacks putAbsent. Rather than split it into a "get, then put" check, we do it as follows,
+ // which saves the extra "get" in the no-conflict case [for both normal and super columns],
+ // in exchange for a re-put in the SuperColumn case.
IColumn oldColumn = put(name, column);
if (oldColumn != null)
{
if (oldColumn instanceof SuperColumn)
{
assert column instanceof SuperColumn;
+ // since oldColumn is where we've been accumulating results, it's usually going to be faster to
+ // add the new one to the old, then place old back in the Map, rather than copy the old contents
+ // into the new Map entry.
((SuperColumn) oldColumn).putColumn((SuperColumn)column, allocator);
+ put(name, oldColumn);
}
else
{
Modified: cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java?rev=1199367&r1=1199366&r2=1199367&view=diff
==============================================================================
--- cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java (original)
+++ cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java Tue Nov 8 17:55:16 2011
@@ -51,6 +51,7 @@ import static junit.framework.Assert.ass
import static junit.framework.Assert.assertTrue;
import static org.apache.cassandra.Util.column;
import static org.apache.cassandra.Util.getBytes;
+import static org.apache.cassandra.db.TableTest.assertColumns;
import static org.junit.Assert.assertNull;
import org.junit.Test;
@@ -575,14 +576,14 @@ public class ColumnFamilyStoreTest exten
{
RowMutation rm = new RowMutation(cfs.table.name, key.key);
ColumnFamily cf = ColumnFamily.create(cfs.table.name, cfs.getColumnFamilyName());
- SuperColumn sc = new SuperColumn(scfName, LongType.instance);
+ SuperColumn sc = new SuperColumn(scfName, cfs.metadata.subcolumnComparator);
for (Column col : cols)
sc.addColumn(col);
cf.addColumn(sc);
rm.add(cf);
rm.apply();
}
-
+
private static void putColsStandard(ColumnFamilyStore cfs, DecoratedKey key, Column... cols) throws Throwable
{
RowMutation rm = new RowMutation(cfs.table.name, key.key);
@@ -679,4 +680,28 @@ public class ColumnFamilyStoreTest exten
assertTrue("can not find backedup file:" + desc.filenameFor(c), new File(desc.filenameFor(c)).exists());
}
}
+
+ @Test
+ public void testSuperSliceByNamesCommandOn() throws Throwable
+ {
+ String tableName = "Keyspace1";
+ String cfName= "Super4";
+ ByteBuffer superColName = ByteBufferUtil.bytes("HerpDerp");
+ DecoratedKey key = Util.dk("multiget-slice-resurrection");
+ Table table = Table.open(tableName);
+ ColumnFamilyStore cfs = table.getColumnFamilyStore(cfName);
+
+ // Initially create a SC with 1 subcolumn
+ putColsSuper(cfs, key, superColName, new Column(ByteBufferUtil.bytes("c1"), ByteBufferUtil.bytes("a"), 1));
+ cfs.forceBlockingFlush();
+
+ // Add another column
+ putColsSuper(cfs, key, superColName, new Column(ByteBufferUtil.bytes("c2"), ByteBufferUtil.bytes("b"), 2));
+
+ // Test fetching the supercolumn by name
+ SliceByNamesReadCommand cmd = new SliceByNamesReadCommand(tableName, key.key, new QueryPath(cfName), Collections.singletonList(superColName));
+ ColumnFamily cf = cmd.getRow(table).cf;
+ SuperColumn superColumn = (SuperColumn) cf.getColumn(superColName);
+ assertColumns(superColumn, "c1", "c2");
+ }
}
Modified: cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/ColumnFamilyTest.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/ColumnFamilyTest.java?rev=1199367&r1=1199366&r2=1199367&view=diff
==============================================================================
--- cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/ColumnFamilyTest.java (original)
+++ cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/ColumnFamilyTest.java Tue Nov 8 17:55:16 2011
@@ -30,13 +30,15 @@ import org.junit.Test;
import org.apache.cassandra.io.util.DataOutputBuffer;
import org.apache.cassandra.db.filter.QueryPath;
import static org.apache.cassandra.Util.column;
+import static org.junit.Assert.assertEquals;
+
import org.apache.cassandra.utils.ByteBufferUtil;
import org.apache.cassandra.utils.HeapAllocator;
public class ColumnFamilyTest extends SchemaLoader
{
- // TODO test SuperColumns
+ // TODO test SuperColumns more
@Test
public void testSingleColumn() throws IOException
@@ -146,4 +148,34 @@ public class ColumnFamilyTest extends Sc
cf_result.addColumn(QueryPath.column(ByteBufferUtil.bytes("col3")), ByteBufferUtil.bytes("z"), 2);
assert cf_result.getColumn(ByteBufferUtil.bytes("col3")).value().equals(ByteBufferUtil.bytes("z"));
}
+
+ private void testSuperColumnResolution(ISortedColumns.Factory factory)
+ {
+ ColumnFamilyStore cfs = Table.open("Keyspace1").getColumnFamilyStore("Super1");
+ ColumnFamily cf = ColumnFamily.create(cfs.metadata, factory);
+ ByteBuffer superColumnName = ByteBufferUtil.bytes("sc");
+ ByteBuffer subColumnName = ByteBufferUtil.bytes(1L);
+
+ Column first = new Column(subColumnName, ByteBufferUtil.bytes("one"), 1L);
+ Column second = new Column(subColumnName, ByteBufferUtil.bytes("two"), 2L);
+
+ cf.addColumn(superColumnName, first);
+
+ // resolve older + new
+ cf.addColumn(superColumnName, second);
+ assertEquals(second, cf.getColumn(superColumnName).getSubColumn(subColumnName));
+
+ // resolve new + older
+ cf.addColumn(superColumnName, first);
+ assertEquals(second, cf.getColumn(superColumnName).getSubColumn(subColumnName));
+ }
+
+ @Test
+ public void testSuperColumnResolution()
+ {
+ testSuperColumnResolution(TreeMapBackedSortedColumns.factory());
+ testSuperColumnResolution(ThreadSafeSortedColumns.factory());
+ // array-sorted does allow conflict resolution IF it is the last column. Bit of an edge case.
+ testSuperColumnResolution(ArrayBackedSortedColumns.factory());
+ }
}
Modified: cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/TableTest.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/TableTest.java?rev=1199367&r1=1199366&r2=1199367&view=diff
==============================================================================
--- cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/TableTest.java (original)
+++ cassandra/branches/cassandra-1.0/test/unit/org/apache/cassandra/db/TableTest.java Tue Nov 8 17:55:16 2011
@@ -542,17 +542,10 @@ public class TableTest extends CleanupHe
String[] columnNames1 = names.toArray(new String[0]);
String[] la = L.toArray(new String[columns.size()]);
- StringBuffer lasb = new StringBuffer();
- for (String l: la)
- {
- lasb.append(l);
- lasb.append(", ");
- }
assert Arrays.equals(la, columnNames1)
- : String.format("Columns [%s(as string: %s)])] is not expected [%s]",
+ : String.format("Columns [%s])] is not expected [%s]",
((container == null) ? "" : container.getComparator().getColumnsString(columns)),
- lasb.toString(),
StringUtils.join(columnNames1, ","));
}
@@ -573,6 +566,4 @@ public class TableTest extends CleanupHe
assertEquals(0, ByteBufferUtil.compareUnsigned(column.value(), ByteBufferUtil.bytes(value)));
assertEquals(timestamp, column.timestamp());
}
-
-
}