PHOENIX-2546 Filters should take into account that multiple versions may be scanned
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/267715aa Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/267715aa Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/267715aa Branch: refs/heads/4.x-HBase-1.0 Commit: 267715aae9022b21c664e3721c2fbf1e6bf5fde6 Parents: ca10171 Author: James Taylor <[email protected]> Authored: Tue Dec 29 23:00:27 2015 -0800 Committer: James Taylor <[email protected]> Committed: Wed Dec 30 18:14:25 2015 -0800 ---------------------------------------------------------------------- .../org/apache/phoenix/tx/TransactionIT.java | 147 +++++++++++++++++++ .../phoenix/filter/ColumnProjectionFilter.java | 3 +- .../MultiCFCQKeyValueComparisonFilter.java | 1 - .../filter/MultiCQKeyValueComparisonFilter.java | 2 - .../filter/MultiKeyValueComparisonFilter.java | 69 ++++----- .../phoenix/filter/RowKeyComparisonFilter.java | 11 +- .../SingleCFCQKeyValueComparisonFilter.java | 3 - .../SingleCQKeyValueComparisonFilter.java | 3 - .../filter/SingleKeyValueComparisonFilter.java | 19 +-- .../apache/phoenix/filter/SkipScanFilter.java | 6 +- .../schema/tuple/SingleKeyValueTuple.java | 46 +++--- .../phoenix/filter/SkipScanFilterTest.java | 2 +- 12 files changed, 217 insertions(+), 95 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/267715aa/phoenix-core/src/it/java/org/apache/phoenix/tx/TransactionIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/tx/TransactionIT.java b/phoenix-core/src/it/java/org/apache/phoenix/tx/TransactionIT.java index e83467a..63a5d6e 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/tx/TransactionIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/tx/TransactionIT.java @@ -62,6 +62,7 @@ import org.apache.phoenix.util.ReadOnlyProps; import org.apache.phoenix.util.TestUtil; import org.junit.Before; import org.junit.BeforeClass; +import org.junit.Ignore; import org.junit.Test; import com.google.common.collect.Lists; @@ -350,6 +351,7 @@ public class TransactionIT extends BaseHBaseManagedTimeIT { assertFalse(rs.next()); } + @Ignore @Test public void testNonTxToTxTableFailure() throws Exception { Connection conn = DriverManager.getConnection(getUrl()); @@ -725,4 +727,149 @@ public class TransactionIT extends BaseHBaseManagedTimeIT { Result result = htable.get(new Get(Bytes.toBytes("j"))); assertTrue(result.isEmpty()); } + + @Test + public void testCheckpointAndRollback() throws Exception { + Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES); + Connection conn = DriverManager.getConnection(getUrl(), props); + conn.setAutoCommit(false); + try { + String fullTableName = "T"; + Statement stmt = conn.createStatement(); + stmt.execute("CREATE TABLE " + fullTableName + "(k VARCHAR PRIMARY KEY, v1 VARCHAR, v2 VARCHAR) TRANSACTIONAL=true"); + stmt.executeUpdate("upsert into " + fullTableName + " values('x', 'a', 'a')"); + conn.commit(); + + stmt.executeUpdate("upsert into " + fullTableName + "(k,v1) SELECT k,v1||'a' FROM " + fullTableName); + ResultSet rs = stmt.executeQuery("select k, v1, v2 from " + fullTableName); + assertTrue(rs.next()); + assertEquals("x", rs.getString(1)); + assertEquals("aa", rs.getString(2)); + assertEquals("a", rs.getString(3)); + assertFalse(rs.next()); + + stmt.executeUpdate("upsert into " + fullTableName + "(k,v1) SELECT k,v1||'a' FROM " + fullTableName); + + rs = stmt.executeQuery("select k, v1, v2 from " + fullTableName); + assertTrue(rs.next()); + assertEquals("x", rs.getString(1)); + assertEquals("aaa", rs.getString(2)); + assertEquals("a", rs.getString(3)); + assertFalse(rs.next()); + + conn.rollback(); + + //assert original row exists in fullTableName1 + rs = stmt.executeQuery("select k, v1, v2 from " + fullTableName); + assertTrue(rs.next()); + assertEquals("x", rs.getString(1)); + assertEquals("a", rs.getString(2)); + assertEquals("a", rs.getString(3)); + assertFalse(rs.next()); + + } finally { + conn.close(); + } + } + + @Ignore("Add back once TEPHRA-162 gets fixed") + @Test + public void testInflightUpdateNotSeen() throws Exception { + String selectSQL = "SELECT * FROM " + FULL_TABLE_NAME; + try (Connection conn1 = DriverManager.getConnection(getUrl()); + Connection conn2 = DriverManager.getConnection(getUrl())) { + conn1.setAutoCommit(false); + conn2.setAutoCommit(true); + ResultSet rs = conn1.createStatement().executeQuery(selectSQL); + assertFalse(rs.next()); + + String upsert = "UPSERT INTO " + FULL_TABLE_NAME + "(varchar_pk, char_pk, int_pk, long_pk, decimal_pk, date_pk) VALUES(?, ?, ?, ?, ?, ?)"; + PreparedStatement stmt = conn1.prepareStatement(upsert); + // upsert two rows + TestUtil.setRowKeyColumns(stmt, 1); + stmt.execute(); + conn1.commit(); + + TestUtil.setRowKeyColumns(stmt, 2); + stmt.execute(); + + rs = conn1.createStatement().executeQuery("SELECT count(*) FROM " + FULL_TABLE_NAME + " WHERE int_col1 IS NULL"); + assertTrue(rs.next()); + assertEquals(2, rs.getInt(1)); + + upsert = "UPSERT INTO " + FULL_TABLE_NAME + "(varchar_pk, char_pk, int_pk, long_pk, decimal_pk, date_pk, int_col1) VALUES(?, ?, ?, ?, ?, ?, 1)"; + stmt = conn1.prepareStatement(upsert); + TestUtil.setRowKeyColumns(stmt, 1); + stmt.execute(); + + rs = conn1.createStatement().executeQuery("SELECT int_col1 FROM " + FULL_TABLE_NAME + " WHERE int_col1 = 1"); + assertTrue(rs.next()); + assertEquals(1, rs.getInt(1)); + assertFalse(rs.next()); + + rs = conn2.createStatement().executeQuery("SELECT count(*) FROM " + FULL_TABLE_NAME + " WHERE int_col1 = 1"); + assertTrue(rs.next()); + assertEquals(0, rs.getInt(1)); + rs = conn2.createStatement().executeQuery("SELECT * FROM " + FULL_TABLE_NAME + " WHERE int_col1 = 1"); + assertFalse(rs.next()); + + conn1.commit(); + + rs = conn2.createStatement().executeQuery("SELECT count(*) FROM " + FULL_TABLE_NAME + " WHERE int_col1 = 1"); + assertTrue(rs.next()); + assertEquals(1, rs.getInt(1)); + rs = conn2.createStatement().executeQuery("SELECT * FROM " + FULL_TABLE_NAME + " WHERE int_col1 = 1"); + assertTrue(rs.next()); + assertFalse(rs.next()); + } + } + + @Ignore("Add back once TEPHRA-162 gets fixed") + @Test + public void testInflightDeleteNotSeen() throws Exception { + String selectSQL = "SELECT * FROM " + FULL_TABLE_NAME; + try (Connection conn1 = DriverManager.getConnection(getUrl()); + Connection conn2 = DriverManager.getConnection(getUrl())) { + conn1.setAutoCommit(false); + conn2.setAutoCommit(true); + ResultSet rs = conn1.createStatement().executeQuery(selectSQL); + assertFalse(rs.next()); + + String upsert = "UPSERT INTO " + FULL_TABLE_NAME + "(varchar_pk, char_pk, int_pk, long_pk, decimal_pk, date_pk) VALUES(?, ?, ?, ?, ?, ?)"; + PreparedStatement stmt = conn1.prepareStatement(upsert); + // upsert two rows + TestUtil.setRowKeyColumns(stmt, 1); + stmt.execute(); + TestUtil.setRowKeyColumns(stmt, 2); + stmt.execute(); + + conn1.commit(); + + rs = conn1.createStatement().executeQuery("SELECT count(*) FROM " + FULL_TABLE_NAME); + assertTrue(rs.next()); + assertEquals(2, rs.getInt(1)); + + String delete = "DELETE FROM " + FULL_TABLE_NAME + " WHERE varchar_pk = 'varchar1'"; + stmt = conn1.prepareStatement(delete); + int count = stmt.executeUpdate(); + assertEquals(1,count); + + rs = conn1.createStatement().executeQuery("SELECT count(*) FROM " + FULL_TABLE_NAME); + assertTrue(rs.next()); + assertEquals(1, rs.getInt(1)); + assertFalse(rs.next()); + + rs = conn2.createStatement().executeQuery("SELECT count(*) FROM " + FULL_TABLE_NAME); + assertTrue(rs.next()); + assertEquals(2, rs.getInt(1)); + assertFalse(rs.next()); + + conn1.commit(); + + rs = conn2.createStatement().executeQuery("SELECT count(*) FROM " + FULL_TABLE_NAME); + assertTrue(rs.next()); + assertEquals(1, rs.getInt(1)); + assertFalse(rs.next()); + } + } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/267715aa/phoenix-core/src/main/java/org/apache/phoenix/filter/ColumnProjectionFilter.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/filter/ColumnProjectionFilter.java b/phoenix-core/src/main/java/org/apache/phoenix/filter/ColumnProjectionFilter.java index a238e8e..cf9f7ab 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/filter/ColumnProjectionFilter.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/filter/ColumnProjectionFilter.java @@ -33,7 +33,6 @@ import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.KeyValue; import org.apache.hadoop.hbase.KeyValue.Type; -import org.apache.hadoop.hbase.KeyValueUtil; import org.apache.hadoop.hbase.exceptions.DeserializationException; import org.apache.hadoop.hbase.filter.FilterBase; import org.apache.hadoop.hbase.util.Bytes; @@ -175,6 +174,6 @@ public class ColumnProjectionFilter extends FilterBase implements Writable { @Override public ReturnCode filterKeyValue(Cell ignored) throws IOException { - return ReturnCode.INCLUDE; + return ReturnCode.INCLUDE_AND_NEXT_COL; } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/phoenix/blob/267715aa/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiCFCQKeyValueComparisonFilter.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiCFCQKeyValueComparisonFilter.java b/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiCFCQKeyValueComparisonFilter.java index 9147f1a..3bd1fd7 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiCFCQKeyValueComparisonFilter.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiCFCQKeyValueComparisonFilter.java @@ -31,7 +31,6 @@ import org.apache.phoenix.expression.Expression; * are references to multiple column qualifiers over multiple column families. * Also there same qualifier names in different families. * - * @since 0.1 */ public class MultiCFCQKeyValueComparisonFilter extends MultiKeyValueComparisonFilter { private final ImmutablePairBytesPtr ptr = new ImmutablePairBytesPtr(); http://git-wip-us.apache.org/repos/asf/phoenix/blob/267715aa/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiCQKeyValueComparisonFilter.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiCQKeyValueComparisonFilter.java b/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiCQKeyValueComparisonFilter.java index 5fa5035..91e4392 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiCQKeyValueComparisonFilter.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiCQKeyValueComparisonFilter.java @@ -29,8 +29,6 @@ import org.apache.phoenix.hbase.index.util.ImmutableBytesPtr; * Filter that evaluates WHERE clause expression, used in the case where there * are references to multiple unique column qualifiers over one or more column families. * - * - * @since 0.1 */ public class MultiCQKeyValueComparisonFilter extends MultiKeyValueComparisonFilter { private ImmutableBytesPtr ptr = new ImmutableBytesPtr(); http://git-wip-us.apache.org/repos/asf/phoenix/blob/267715aa/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiKeyValueComparisonFilter.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiKeyValueComparisonFilter.java b/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiKeyValueComparisonFilter.java index 1cb2255..569faa5 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiKeyValueComparisonFilter.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/filter/MultiKeyValueComparisonFilter.java @@ -24,8 +24,6 @@ import java.util.Map; import java.util.TreeSet; import org.apache.hadoop.hbase.Cell; -import org.apache.hadoop.hbase.KeyValue; -import org.apache.hadoop.hbase.KeyValueUtil; import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.hadoop.hbase.util.Bytes; import org.apache.phoenix.expression.Expression; @@ -41,8 +39,6 @@ import org.apache.phoenix.schema.tuple.BaseTuple; * but for general expression evaluation in the case where multiple KeyValue * columns are referenced in the expression. * - * - * @since 0.1 */ public abstract class MultiKeyValueComparisonFilter extends BooleanExpressionFilter { private static final byte[] UNITIALIZED_KEY_BUFFER = new byte[0]; @@ -59,14 +55,14 @@ public abstract class MultiKeyValueComparisonFilter extends BooleanExpressionFil init(); } - private static final class KeyValueRef { - public KeyValue keyValue; + private static final class CellRef { + public Cell cell; @Override public String toString() { - if(keyValue != null) { - return keyValue.toString() + " value = " + Bytes.toStringBinary( - keyValue.getValueArray(), keyValue.getValueOffset(), keyValue.getValueLength()); + if(cell != null) { + return cell.toString() + " value = " + Bytes.toStringBinary( + cell.getValueArray(), cell.getValueOffset(), cell.getValueLength()); } else { return super.toString(); } @@ -79,13 +75,13 @@ public abstract class MultiKeyValueComparisonFilter extends BooleanExpressionFil private final class IncrementalResultTuple extends BaseTuple { private int refCount; private final ImmutableBytesWritable keyPtr = new ImmutableBytesWritable(UNITIALIZED_KEY_BUFFER); - private final Map<Object,KeyValueRef> foundColumns = new HashMap<Object,KeyValueRef>(5); + private final Map<Object,CellRef> foundColumns = new HashMap<Object,CellRef>(5); public void reset() { refCount = 0; keyPtr.set(UNITIALIZED_KEY_BUFFER); - for (KeyValueRef ref : foundColumns.values()) { - ref.keyValue = null; + for (CellRef ref : foundColumns.values()) { + ref.cell = null; } } @@ -98,39 +94,39 @@ public abstract class MultiKeyValueComparisonFilter extends BooleanExpressionFil refCount = foundColumns.size(); } - public ReturnCode resolveColumn(KeyValue value) { + public ReturnCode resolveColumn(Cell value) { // Always set key, in case we never find a key value column of interest, // and our expression uses row key columns. setKey(value); Object ptr = setColumnKey(value.getFamilyArray(), value.getFamilyOffset(), value.getFamilyLength(), value.getQualifierArray(), value.getQualifierOffset(), value.getQualifierLength()); - KeyValueRef ref = foundColumns.get(ptr); + CellRef ref = foundColumns.get(ptr); if (ref == null) { - // Return INCLUDE here. Although this filter doesn't need this KV + // Return INCLUDE_AND_NEXT_COL here. Although this filter doesn't need this KV // it should still be projected into the Result - return ReturnCode.INCLUDE; + return ReturnCode.INCLUDE_AND_NEXT_COL; } // Since we only look at the latest key value for a given column, // we are not interested in older versions // TODO: test with older versions to confirm this doesn't get tripped // This shouldn't be necessary, because a scan only looks at the latest // version - if (ref.keyValue != null) { + if (ref.cell != null) { // Can't do NEXT_ROW, because then we don't match the other columns // SKIP, INCLUDE, and NEXT_COL seem to all act the same return ReturnCode.NEXT_COL; } - ref.keyValue = value; + ref.cell = value; refCount++; return null; } public void addColumn(byte[] cf, byte[] cq) { Object ptr = MultiKeyValueComparisonFilter.this.newColumnKey(cf, 0, cf.length, cq, 0, cq.length); - foundColumns.put(ptr, new KeyValueRef()); + foundColumns.put(ptr, new CellRef()); } - public void setKey(KeyValue value) { + public void setKey(Cell value) { keyPtr.set(value.getRowArray(), value.getRowOffset(), value.getRowLength()); } @@ -140,10 +136,10 @@ public abstract class MultiKeyValueComparisonFilter extends BooleanExpressionFil } @Override - public KeyValue getValue(byte[] cf, byte[] cq) { + public Cell getValue(byte[] cf, byte[] cq) { Object ptr = setColumnKey(cf, 0, cf.length, cq, 0, cq.length); - KeyValueRef ref = foundColumns.get(ptr); - return ref == null ? null : ref.keyValue; + CellRef ref = foundColumns.get(ptr); + return ref == null ? null : ref.cell; } @Override @@ -157,15 +153,15 @@ public abstract class MultiKeyValueComparisonFilter extends BooleanExpressionFil } @Override - public KeyValue getValue(int index) { + public Cell getValue(int index) { // This won't perform very well, but it's not // currently used anyway - for (KeyValueRef ref : foundColumns.values()) { - if (ref.keyValue == null) { + for (CellRef ref : foundColumns.values()) { + if (ref.cell == null) { continue; } if (index == 0) { - return ref.keyValue; + return ref.cell; } index--; } @@ -175,10 +171,10 @@ public abstract class MultiKeyValueComparisonFilter extends BooleanExpressionFil @Override public boolean getValue(byte[] family, byte[] qualifier, ImmutableBytesWritable ptr) { - KeyValue kv = getValue(family, qualifier); - if (kv == null) + Cell cell = getValue(family, qualifier); + if (cell == null) return false; - ptr.set(kv.getValueArray(), kv.getValueOffset(), kv.getValueLength()); + ptr.set(cell.getValueArray(), cell.getValueOffset(), cell.getValueLength()); return true; } } @@ -197,17 +193,17 @@ public abstract class MultiKeyValueComparisonFilter extends BooleanExpressionFil } @Override - public ReturnCode filterKeyValue(Cell keyValue) { + public ReturnCode filterKeyValue(Cell cell) { if (Boolean.TRUE.equals(this.matchedColumn)) { // We already found and matched the single column, all keys now pass - return ReturnCode.INCLUDE; + return ReturnCode.INCLUDE_AND_NEXT_COL; } if (Boolean.FALSE.equals(this.matchedColumn)) { // We found all the columns, but did not match the expression, so skip to next row return ReturnCode.NEXT_ROW; } // This is a key value we're not interested in (TODO: why INCLUDE here instead of NEXT_COL?) - ReturnCode code = inputTuple.resolveColumn(KeyValueUtil.ensureKeyValue(keyValue)); + ReturnCode code = inputTuple.resolveColumn(cell); if (code != null) { return code; } @@ -220,10 +216,10 @@ public abstract class MultiKeyValueComparisonFilter extends BooleanExpressionFil if (inputTuple.isImmutable()) { this.matchedColumn = Boolean.FALSE; } else { - return ReturnCode.INCLUDE; + return ReturnCode.INCLUDE_AND_NEXT_COL; } } - return this.matchedColumn ? ReturnCode.INCLUDE : ReturnCode.NEXT_ROW; + return this.matchedColumn ? ReturnCode.INCLUDE_AND_NEXT_COL : ReturnCode.NEXT_ROW; } @Override @@ -243,8 +239,7 @@ public abstract class MultiKeyValueComparisonFilter extends BooleanExpressionFil super.reset(); } - @SuppressWarnings("all") - // suppressing missing @Override since this doesn't exist for HBase 0.94.4 + @Override public boolean isFamilyEssential(byte[] name) { // Only the column families involved in the expression are essential. // The others are for columns projected in the select expression. http://git-wip-us.apache.org/repos/asf/phoenix/blob/267715aa/phoenix-core/src/main/java/org/apache/phoenix/filter/RowKeyComparisonFilter.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/filter/RowKeyComparisonFilter.java b/phoenix-core/src/main/java/org/apache/phoenix/filter/RowKeyComparisonFilter.java index b7de7ac..2eb69bd 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/filter/RowKeyComparisonFilter.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/filter/RowKeyComparisonFilter.java @@ -22,7 +22,6 @@ import java.io.DataOutput; import java.io.IOException; import org.apache.hadoop.hbase.Cell; -import org.apache.hadoop.hbase.KeyValue; import org.apache.hadoop.hbase.exceptions.DeserializationException; import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.hadoop.hbase.util.Bytes; @@ -38,8 +37,6 @@ import org.slf4j.LoggerFactory; * * Filter for use when expressions only reference row key columns * - * - * @since 0.1 */ public class RowKeyComparisonFilter extends BooleanExpressionFilter { private static final Logger logger = LoggerFactory.getLogger(RowKeyComparisonFilter.class); @@ -79,7 +76,7 @@ public class RowKeyComparisonFilter extends BooleanExpressionFilter { } evaluate = false; } - return keepRow ? ReturnCode.INCLUDE : ReturnCode.NEXT_ROW; + return keepRow ? ReturnCode.INCLUDE_AND_NEXT_COL : ReturnCode.NEXT_ROW; } private final class RowKeyTuple extends BaseTuple { @@ -99,7 +96,7 @@ public class RowKeyComparisonFilter extends BooleanExpressionFilter { } @Override - public KeyValue getValue(byte[] cf, byte[] cq) { + public Cell getValue(byte[] cf, byte[] cq) { return null; } @@ -119,7 +116,7 @@ public class RowKeyComparisonFilter extends BooleanExpressionFilter { } @Override - public KeyValue getValue(int index) { + public Cell getValue(int index) { throw new IndexOutOfBoundsException(Integer.toString(index)); } @@ -135,7 +132,7 @@ public class RowKeyComparisonFilter extends BooleanExpressionFilter { return !this.keepRow; } - @SuppressWarnings("all") // suppressing missing @Override since this doesn't exist for HBase 0.94.4 + @Override public boolean isFamilyEssential(byte[] name) { // We only need our "guaranteed to have a key value" column family, // which we pass in and serialize through. In the case of a VIEW where http://git-wip-us.apache.org/repos/asf/phoenix/blob/267715aa/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleCFCQKeyValueComparisonFilter.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleCFCQKeyValueComparisonFilter.java b/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleCFCQKeyValueComparisonFilter.java index 963fe59..c63673c 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleCFCQKeyValueComparisonFilter.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleCFCQKeyValueComparisonFilter.java @@ -22,7 +22,6 @@ import java.io.IOException; import org.apache.hadoop.hbase.exceptions.DeserializationException; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Writables; - import org.apache.phoenix.expression.Expression; @@ -32,8 +31,6 @@ import org.apache.phoenix.expression.Expression; * column qualifier parts of the key value to disambiguate with another similarly * named column qualifier in a different column family. * - * - * @since 0.1 */ public class SingleCFCQKeyValueComparisonFilter extends SingleKeyValueComparisonFilter { public SingleCFCQKeyValueComparisonFilter() { http://git-wip-us.apache.org/repos/asf/phoenix/blob/267715aa/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleCQKeyValueComparisonFilter.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleCQKeyValueComparisonFilter.java b/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleCQKeyValueComparisonFilter.java index 240c8a6..0d904bc 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleCQKeyValueComparisonFilter.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleCQKeyValueComparisonFilter.java @@ -22,7 +22,6 @@ import java.io.IOException; import org.apache.hadoop.hbase.exceptions.DeserializationException; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Writables; - import org.apache.phoenix.expression.Expression; @@ -32,8 +31,6 @@ import org.apache.phoenix.expression.Expression; * part of the key value since the column qualifier is unique across all column * families. * - * - * @since 0.1 */ public class SingleCQKeyValueComparisonFilter extends SingleKeyValueComparisonFilter { public SingleCQKeyValueComparisonFilter() { http://git-wip-us.apache.org/repos/asf/phoenix/blob/267715aa/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleKeyValueComparisonFilter.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleKeyValueComparisonFilter.java b/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleKeyValueComparisonFilter.java index 8929f8a..eaf8d35 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleKeyValueComparisonFilter.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/filter/SingleKeyValueComparisonFilter.java @@ -21,7 +21,6 @@ import java.io.DataInput; import java.io.IOException; import org.apache.hadoop.hbase.Cell; -import org.apache.hadoop.hbase.KeyValueUtil; import org.apache.hadoop.hbase.util.Bytes; import org.apache.phoenix.expression.Expression; import org.apache.phoenix.expression.KeyValueColumnExpression; @@ -37,8 +36,6 @@ import org.apache.phoenix.schema.tuple.SingleKeyValueTuple; * but for general expression evaluation in the case where only a single KeyValue * column is referenced in the expression. * - * - * @since 0.1 */ public abstract class SingleKeyValueComparisonFilter extends BooleanExpressionFilter { private final SingleKeyValueTuple inputTuple = new SingleKeyValueTuple(); @@ -76,8 +73,7 @@ public abstract class SingleKeyValueComparisonFilter extends BooleanExpressionFi public ReturnCode filterKeyValue(Cell keyValue) { if (this.matchedColumn) { // We already found and matched the single column, all keys now pass - // TODO: why won't this cause earlier versions of a kv to be included? - return ReturnCode.INCLUDE; + return ReturnCode.INCLUDE_AND_NEXT_COL; } if (this.foundColumn()) { // We found all the columns, but did not match the expression, so skip to next row @@ -87,19 +83,18 @@ public abstract class SingleKeyValueComparisonFilter extends BooleanExpressionFi keyValue.getQualifierArray(), keyValue.getQualifierOffset(), keyValue.getQualifierLength()) != 0) { // Remember the key in case this is the only key value we see. // We'll need it if we have row key columns too. - inputTuple.setKey(KeyValueUtil.ensureKeyValue(keyValue)); + inputTuple.setKey(keyValue); // This is a key value we're not interested in - // TODO: use NEXT_COL when bug fix comes through that includes the row still - return ReturnCode.INCLUDE; + return ReturnCode.INCLUDE_AND_NEXT_COL; } - inputTuple.setKeyValue(KeyValueUtil.ensureKeyValue(keyValue)); + inputTuple.setCell(keyValue); // We have the columns, so evaluate here if (!Boolean.TRUE.equals(evaluate(inputTuple))) { return ReturnCode.NEXT_ROW; } this.matchedColumn = true; - return ReturnCode.INCLUDE; + return ReturnCode.INCLUDE_AND_NEXT_COL; } @Override @@ -124,7 +119,7 @@ public abstract class SingleKeyValueComparisonFilter extends BooleanExpressionFi return true; } - @Override + @Override public void reset() { inputTuple.reset(); matchedColumn = false; @@ -137,7 +132,7 @@ public abstract class SingleKeyValueComparisonFilter extends BooleanExpressionFi init(); } - @SuppressWarnings("all") // suppressing missing @Override since this doesn't exist for HBase 0.94.4 + @Override public boolean isFamilyEssential(byte[] name) { // Only the column families involved in the expression are essential. // The others are for columns projected in the select expression http://git-wip-us.apache.org/repos/asf/phoenix/blob/267715aa/phoenix-core/src/main/java/org/apache/phoenix/filter/SkipScanFilter.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/filter/SkipScanFilter.java b/phoenix-core/src/main/java/org/apache/phoenix/filter/SkipScanFilter.java index 667b3d7..77b4cf6 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/filter/SkipScanFilter.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/filter/SkipScanFilter.java @@ -274,7 +274,7 @@ public class SkipScanFilter extends FilterBase implements Writable { // more than we need. We can optimize this by tracking whether each range in each slot position // intersects. ReturnCode endCode = navigate(upperExclusiveKey, 0, upperExclusiveKey.length, Terminate.AT); - if (endCode == ReturnCode.INCLUDE) { + if (endCode == ReturnCode.INCLUDE || endCode == ReturnCode.INCLUDE_AND_NEXT_COL) { setStartKey(); // If the upperExclusiveKey is equal to the start key, we've gone one position too far, since // our upper key is exclusive. In that case, go to the previous key @@ -358,7 +358,7 @@ public class SkipScanFilter extends FilterBase implements Writable { // First check to see if we're in-range until we reach our end key if (endKeyLength > 0) { if (Bytes.compareTo(currentKey, offset, length, endKey, 0, endKeyLength) < 0) { - return ReturnCode.INCLUDE; + return ReturnCode.INCLUDE_AND_NEXT_COL; } // If key range of last slot is a single key, we can increment our position @@ -490,7 +490,7 @@ public class SkipScanFilter extends FilterBase implements Writable { // up to the upper range of our last slot. We do this for ranges and single keys // since we potentially have multiple key values for the same row key. setEndKey(ptr, minOffset, i); - return ReturnCode.INCLUDE; + return ReturnCode.INCLUDE_AND_NEXT_COL; } private boolean allTrailingNulls(int i) { http://git-wip-us.apache.org/repos/asf/phoenix/blob/267715aa/phoenix-core/src/main/java/org/apache/phoenix/schema/tuple/SingleKeyValueTuple.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/tuple/SingleKeyValueTuple.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/tuple/SingleKeyValueTuple.java index 8226208..e35eb13 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/tuple/SingleKeyValueTuple.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/tuple/SingleKeyValueTuple.java @@ -17,24 +17,24 @@ */ package org.apache.phoenix.schema.tuple; -import org.apache.hadoop.hbase.KeyValue; +import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.hadoop.hbase.util.Bytes; public class SingleKeyValueTuple extends BaseTuple { private static final byte[] UNITIALIZED_KEY_BUFFER = new byte[0]; - private KeyValue keyValue; + private Cell cell; private ImmutableBytesWritable keyPtr = new ImmutableBytesWritable(UNITIALIZED_KEY_BUFFER); public SingleKeyValueTuple() { } - public SingleKeyValueTuple(KeyValue keyValue) { - if (keyValue == null) { + public SingleKeyValueTuple(Cell cell) { + if (cell == null) { throw new NullPointerException(); } - setKeyValue(keyValue); + setCell(cell); } public boolean hasKey() { @@ -42,28 +42,27 @@ public class SingleKeyValueTuple extends BaseTuple { } public void reset() { - this.keyValue = null; + this.cell = null; keyPtr.set(UNITIALIZED_KEY_BUFFER); } - public void setKeyValue(KeyValue keyValue) { - if (keyValue == null) { + public void setCell(Cell cell) { + if (cell == null) { throw new IllegalArgumentException(); } - this.keyValue = keyValue; - setKey(keyValue); + this.cell = cell; + setKey(cell); } public void setKey(ImmutableBytesWritable ptr) { keyPtr.set(ptr.get(), ptr.getOffset(), ptr.getLength()); } - @SuppressWarnings("deprecation") - public void setKey(KeyValue keyValue) { - if (keyValue == null) { + public void setKey(Cell cell) { + if (cell == null) { throw new IllegalArgumentException(); } - keyPtr.set(keyValue.getBuffer(), keyValue.getRowOffset(), keyValue.getRowLength()); + keyPtr.set(cell.getRowArray(), cell.getRowOffset(), cell.getRowLength()); } @Override @@ -72,8 +71,8 @@ public class SingleKeyValueTuple extends BaseTuple { } @Override - public KeyValue getValue(byte[] cf, byte[] cq) { - return keyValue; + public Cell getValue(byte[] cf, byte[] cq) { + return cell; } @Override @@ -83,29 +82,28 @@ public class SingleKeyValueTuple extends BaseTuple { @Override public String toString() { - return "SingleKeyValueTuple[" + keyValue == null ? keyPtr.get() == UNITIALIZED_KEY_BUFFER ? "null" : Bytes.toStringBinary(keyPtr.get(),keyPtr.getOffset(),keyPtr.getLength()) : keyValue.toString() + "]"; + return "SingleKeyValueTuple[" + cell == null ? keyPtr.get() == UNITIALIZED_KEY_BUFFER ? "null" : Bytes.toStringBinary(keyPtr.get(),keyPtr.getOffset(),keyPtr.getLength()) : cell.toString() + "]"; } @Override public int size() { - return keyValue == null ? 0 : 1; + return cell == null ? 0 : 1; } @Override - public KeyValue getValue(int index) { - if (index != 0 || keyValue == null) { + public Cell getValue(int index) { + if (index != 0 || cell == null) { throw new IndexOutOfBoundsException(Integer.toString(index)); } - return keyValue; + return cell; } - @SuppressWarnings("deprecation") @Override public boolean getValue(byte[] family, byte[] qualifier, ImmutableBytesWritable ptr) { - if (keyValue == null) + if (cell == null) return false; - ptr.set(keyValue.getBuffer(), keyValue.getValueOffset(), keyValue.getValueLength()); + ptr.set(cell.getValueArray(), cell.getValueOffset(), cell.getValueLength()); return true; } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/267715aa/phoenix-core/src/test/java/org/apache/phoenix/filter/SkipScanFilterTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/filter/SkipScanFilterTest.java b/phoenix-core/src/test/java/org/apache/phoenix/filter/SkipScanFilterTest.java index 898f778..4cb71ff 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/filter/SkipScanFilterTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/filter/SkipScanFilterTest.java @@ -402,7 +402,7 @@ public class SkipScanFilterTest extends TestCase { skipper.reset(); assertFalse(skipper.filterAllRemaining()); assertFalse(skipper.filterRowKey(kv.getBuffer(), kv.getRowOffset(), kv.getRowLength())); - assertEquals(kv.toString(), ReturnCode.INCLUDE, skipper.filterKeyValue(kv)); + assertEquals(kv.toString(), ReturnCode.INCLUDE_AND_NEXT_COL, skipper.filterKeyValue(kv)); } @Override public String toString() {
