You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ch...@apache.org on 2017/03/04 02:47:22 UTC
phoenix git commit: PHOENIX-3705 SkipScanFilter may repeatedly copy
rowKey Columns to startKey
Repository: phoenix
Updated Branches:
refs/heads/master cf65fb27e -> c8612fa1b
PHOENIX-3705 SkipScanFilter may repeatedly copy rowKey Columns to startKey
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/c8612fa1
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/c8612fa1
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/c8612fa1
Branch: refs/heads/master
Commit: c8612fa1b09f883726102951626798244f73db17
Parents: cf65fb2
Author: chenglei <ch...@apache.org>
Authored: Sat Mar 4 10:45:57 2017 +0800
Committer: chenglei <ch...@apache.org>
Committed: Sat Mar 4 10:45:57 2017 +0800
----------------------------------------------------------------------
.../apache/phoenix/filter/SkipScanFilter.java | 15 +-
.../org/apache/phoenix/schema/RowKeySchema.java | 10 +-
.../phoenix/filter/SkipScanFilterTest.java | 229 ++++++++++++++++++-
3 files changed, 245 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/phoenix/blob/c8612fa1/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 c966d91..c9d951c 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
@@ -445,13 +445,26 @@ public class SkipScanFilter extends FilterBase implements Writable {
setStartKey();
schema.reposition(ptr, ScanUtil.getRowKeyPosition(slotSpan, i), ScanUtil.getRowKeyPosition(slotSpan, j), minOffset, maxOffset, slotSpan[j]);
} else {
+ //for PHOENIX-3705, now ptr is still point to slot i, we must make ptr point to slot j+1,
+ //because following setStartKey method will copy rowKey columns before ptr to startKey and
+ //then copy the lower bound of slots from j+1, according to position array, so if we do not
+ //make ptr point to slot j+1 before setStartKey,the startKey would be erroneous.
+ schema.reposition(
+ ptr,
+ ScanUtil.getRowKeyPosition(slotSpan, i),
+ ScanUtil.getRowKeyPosition(slotSpan, j + 1),
+ minOffset,
+ maxOffset,
+ slotSpan[j + 1]);
int currentLength = setStartKey(ptr, minOffset, j+1, nSlots, false);
// From here on, we use startKey as our buffer (resetting minOffset and maxOffset)
// We've copied the part of the current key above that we need into startKey
// Reinitialize the iterator to be positioned at previous slot position
minOffset = 0;
maxOffset = startKeyLength;
- schema.iterator(startKey, minOffset, maxOffset, ptr, ScanUtil.getRowKeyPosition(slotSpan, j)+1);
+ //make ptr point to the first rowKey column of slot j,why we need slotSpan[j] because for Row Value Constructor(RVC),
+ //slot j may span multiple rowKey columns, so the length of ptr must consider the slotSpan[j].
+ schema.iterator(startKey, minOffset, maxOffset, ptr, ScanUtil.getRowKeyPosition(slotSpan, j)+1,slotSpan[j]);
// Do nextKey after setting the accessor b/c otherwise the null byte may have
// been incremented causing us not to find it
ByteUtil.nextKey(startKey, currentLength);
http://git-wip-us.apache.org/repos/asf/phoenix/blob/c8612fa1/phoenix-core/src/main/java/org/apache/phoenix/schema/RowKeySchema.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/RowKeySchema.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/RowKeySchema.java
index 9d86dd6..3fa3a36 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/schema/RowKeySchema.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/RowKeySchema.java
@@ -81,19 +81,25 @@ public class RowKeySchema extends ValueSchema {
}
// "iterator" initialization methods that initialize a bytes ptr with a row key for further navigation
-
@edu.umd.cs.findbugs.annotations.SuppressWarnings(
value="NP_BOOLEAN_RETURN_NULL",
justification="Designed to return null.")
- public Boolean iterator(byte[] src, int srcOffset, int srcLength, ImmutableBytesWritable ptr, int position) {
+ public Boolean iterator(byte[] src, int srcOffset, int srcLength, ImmutableBytesWritable ptr, int position,int extraColumnSpan) {
Boolean hasValue = null;
ptr.set(src, srcOffset, 0);
int maxOffset = srcOffset + srcLength;
for (int i = 0; i < position; i++) {
hasValue = next(ptr, i, maxOffset);
}
+ if(extraColumnSpan > 0) {
+ readExtraFields(ptr, position, maxOffset, extraColumnSpan);
+ }
return hasValue;
}
+
+ public Boolean iterator(byte[] src, int srcOffset, int srcLength, ImmutableBytesWritable ptr, int position) {
+ return iterator(src, srcOffset,srcLength, ptr, position,0);
+ }
public Boolean iterator(ImmutableBytesWritable srcPtr, ImmutableBytesWritable ptr, int position) {
return iterator(srcPtr.get(), srcPtr.getOffset(), srcPtr.getLength(), ptr, position);
http://git-wip-us.apache.org/repos/asf/phoenix/blob/c8612fa1/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 d691535..6c28cdf 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
@@ -33,6 +33,7 @@ import org.apache.phoenix.schema.RowKeySchema.RowKeySchemaBuilder;
import org.apache.phoenix.schema.SortOrder;
import org.apache.phoenix.schema.types.PChar;
import org.apache.phoenix.schema.types.PDataType;
+import org.apache.phoenix.schema.types.PInteger;
import org.apache.phoenix.schema.types.PVarchar;
import org.apache.phoenix.util.ByteUtil;
import org.junit.Test;
@@ -57,7 +58,7 @@ public class SkipScanFilterTest extends TestCase {
private final List<List<KeyRange>> cnf;
private final List<Expectation> expectations;
- public SkipScanFilterTest(List<List<KeyRange>> cnf, int[] widths, List<Expectation> expectations) {
+ public SkipScanFilterTest(List<List<KeyRange>> cnf, int[] widths, int[] slotSpans,List<Expectation> expectations) {
this.expectations = expectations;
this.cnf = cnf;
RowKeySchemaBuilder builder = new RowKeySchemaBuilder(widths.length);
@@ -92,7 +93,11 @@ public class SkipScanFilterTest extends TestCase {
}, width <= 0, SortOrder.getDefault());
}
- skipper = new SkipScanFilter(cnf, builder.build());
+ if(slotSpans==null) {
+ skipper = new SkipScanFilter(cnf, builder.build());
+ } else {
+ skipper = new SkipScanFilter(cnf, slotSpans,builder.build());
+ }
}
@Test
@@ -102,7 +107,7 @@ public class SkipScanFilterTest extends TestCase {
}
}
- @Parameters(name="{0} {1} {2}")
+ @Parameters(name="{0} {1} {3}")
public static Collection<Object> data() {
List<Object> testCases = Lists.newArrayList();
// Variable length tests
@@ -122,6 +127,7 @@ public class SkipScanFilterTest extends TestCase {
PVarchar.INSTANCE.getKeyRange(Bytes.toBytes("1"), true, Bytes.toBytes("1"), true),
}},
new int[4],
+ null,
new Include(ByteUtil.concat(Bytes.toBytes("a"),QueryConstants.SEPARATOR_BYTE_ARRAY,
Bytes.toBytes("b"), QueryConstants.SEPARATOR_BYTE_ARRAY,
QueryConstants.SEPARATOR_BYTE_ARRAY,
@@ -151,6 +157,7 @@ public class SkipScanFilterTest extends TestCase {
KeyRange.EVERYTHING_RANGE,
}*/},
new int[4],
+ null,
new SeekNext(ByteUtil.concat(Bytes.toBytes("20160116141006"),QueryConstants.SEPARATOR_BYTE_ARRAY,
QueryConstants.SEPARATOR_BYTE_ARRAY,
Bytes.toBytes("servlet") ),
@@ -179,6 +186,7 @@ public class SkipScanFilterTest extends TestCase {
PChar.INSTANCE.getKeyRange(Bytes.toBytes("AA"), true, Bytes.toBytes("AB"), false),
}},
new int[]{3,2,2,2,2},
+ null,
new SeekNext("defAAABABAB", "dzzAAAAAAAA"),
new Finished("xyyABABABAB"))
);
@@ -187,6 +195,7 @@ public class SkipScanFilterTest extends TestCase {
PVarchar.INSTANCE.getKeyRange(Bytes.toBytes("j"), false, Bytes.toBytes("k"), true),
}},
new int[]{0},
+ null,
new SeekNext(Bytes.toBytes("a"), ByteUtil.nextKey(new byte[] {'j',QueryConstants.SEPARATOR_BYTE})),
new Include("ja"),
new Include("jz"),
@@ -199,6 +208,7 @@ public class SkipScanFilterTest extends TestCase {
PChar.INSTANCE.getKeyRange(Bytes.toBytes("abc"), true, Bytes.toBytes("def"), true)
}},
new int[]{3},
+ null,
new SeekNext("aab", "aac"),
new SeekNext("abb", "abc"),
new Include("abc"),
@@ -211,6 +221,7 @@ public class SkipScanFilterTest extends TestCase {
PChar.INSTANCE.getKeyRange(Bytes.toBytes("abc"), false, Bytes.toBytes("def"), true)
}},
new int[]{3},
+ null,
new SeekNext("aba", "abd"),
new Include("abe"),
new Include("def"),
@@ -221,6 +232,7 @@ public class SkipScanFilterTest extends TestCase {
PChar.INSTANCE.getKeyRange(Bytes.toBytes("abc"), false, Bytes.toBytes("def"), false)
}},
new int[]{3},
+ null,
new SeekNext("aba", "abd"),
new Finished("def"))
);
@@ -230,6 +242,7 @@ public class SkipScanFilterTest extends TestCase {
PChar.INSTANCE.getKeyRange(Bytes.toBytes("dzy"), false, Bytes.toBytes("xyz"), false),
}},
new int[]{3},
+ null,
new Include("def"),
new SeekNext("deg", "dzz"),
new Include("eee"),
@@ -247,6 +260,7 @@ public class SkipScanFilterTest extends TestCase {
PChar.INSTANCE.getKeyRange(Bytes.toBytes("PO"), true, Bytes.toBytes("PP"), false),
}},
new int[]{3,2},
+ null,
new Include("abcAB"),
new SeekNext("abcAY","abcEB"),
new Include("abcEF"),
@@ -267,6 +281,7 @@ public class SkipScanFilterTest extends TestCase {
PChar.INSTANCE.getKeyRange(Bytes.toBytes("def"), true, Bytes.toBytes("def"), true),
}},
new int[]{2,3},
+ null,
new Include("ABabc"),
new SeekNext("ABdeg","ACabc"),
new Include("AMabc"),
@@ -285,6 +300,7 @@ public class SkipScanFilterTest extends TestCase {
PChar.INSTANCE.getKeyRange(Bytes.toBytes("def"), true, Bytes.toBytes("def"), true),
}},
new int[]{2,3},
+ null,
new Include("POdef"),
new Finished("POdeg"))
);
@@ -296,6 +312,7 @@ public class SkipScanFilterTest extends TestCase {
PChar.INSTANCE.getKeyRange(Bytes.toBytes("def"), true, Bytes.toBytes("def"), true),
}},
new int[]{2,3},
+ null,
new Include("POdef"))
);
testCases.addAll(
@@ -310,6 +327,7 @@ public class SkipScanFilterTest extends TestCase {
PChar.INSTANCE.getKeyRange(Bytes.toBytes("PO"), true, Bytes.toBytes("PP"), false),
}},
new int[]{3,2},
+ null,
new SeekNext("aaaAA", "abcAB"),
new SeekNext("abcZZ", "abdAB"),
new SeekNext("abdZZ", "abeAB"),
@@ -338,6 +356,7 @@ public class SkipScanFilterTest extends TestCase {
PChar.INSTANCE.getKeyRange(Bytes.toBytes("dzz"), true, Bytes.toBytes("xyz"), false),
}},
new int[]{3},
+ null,
new SeekNext("abb", "abc"),
new Include("abc"),
new Include("abe"),
@@ -358,18 +377,212 @@ public class SkipScanFilterTest extends TestCase {
PChar.INSTANCE.getKeyRange(Bytes.toBytes("700"), false, Bytes.toBytes("901"), false),
}},
new int[]{3,2,3},
+ null,
new SeekNext("abcEB700", "abcEB701"),
new Include("abcEB701"),
new SeekNext("dzzAB250", "dzzAB701"),
new Finished("zzzAA000"))
);
+ //for PHOENIX-3705
+ testCases.addAll(
+ foreach(
+ new KeyRange[][]{{
+ PInteger.INSTANCE.getKeyRange(PInteger.INSTANCE.toBytes(1), true, PInteger.INSTANCE.toBytes(4), true)
+ },
+ {
+ KeyRange.getKeyRange(PInteger.INSTANCE.toBytes(5)),
+ KeyRange.getKeyRange(PInteger.INSTANCE.toBytes(7))
+ },
+ {
+ PInteger.INSTANCE.getKeyRange(PInteger.INSTANCE.toBytes(9), true, PInteger.INSTANCE.toBytes(10), true)
+ }},
+ new int[]{4,4,4},
+ null,
+ new SeekNext(
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(2),
+ PInteger.INSTANCE.toBytes(7),
+ PInteger.INSTANCE.toBytes(11)),
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(3),
+ PInteger.INSTANCE.toBytes(5),
+ PInteger.INSTANCE.toBytes(9))),
+ new Finished(ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(4),
+ PInteger.INSTANCE.toBytes(7),
+ PInteger.INSTANCE.toBytes(11))))
+ );
+ testCases.addAll(
+ foreach(
+ new KeyRange[][]{{
+ KeyRange.getKeyRange(PInteger.INSTANCE.toBytes(1)),
+ KeyRange.getKeyRange(PInteger.INSTANCE.toBytes(3)),
+ KeyRange.getKeyRange(PInteger.INSTANCE.toBytes(4))
+ },
+ {
+ KeyRange.getKeyRange(PInteger.INSTANCE.toBytes(5)),
+ KeyRange.getKeyRange(PInteger.INSTANCE.toBytes(7))
+ },
+ {
+ PInteger.INSTANCE.getKeyRange(PInteger.INSTANCE.toBytes(9), true, PInteger.INSTANCE.toBytes(10), true)
+ }},
+ new int[]{4,4,4},
+ null,
+ new SeekNext(
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(3),
+ PInteger.INSTANCE.toBytes(7),
+ PInteger.INSTANCE.toBytes(11)),
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(4),
+ PInteger.INSTANCE.toBytes(5),
+ PInteger.INSTANCE.toBytes(9))),
+ new Finished(ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(4),
+ PInteger.INSTANCE.toBytes(7),
+ PInteger.INSTANCE.toBytes(11))))
+ );
+ //for RVC
+ testCases.addAll(
+ foreach(
+ new KeyRange[][]{
+ {
+ KeyRange.getKeyRange(
+ ByteUtil.concat(PInteger.INSTANCE.toBytes(1),PInteger.INSTANCE.toBytes(2)),
+ true,
+ ByteUtil.concat(PInteger.INSTANCE.toBytes(3),PInteger.INSTANCE.toBytes(4)),
+ true)
+ },
+ {
+ KeyRange.getKeyRange(
+ ByteUtil.concat(PInteger.INSTANCE.toBytes(5),PInteger.INSTANCE.toBytes(6)),
+ true,
+ ByteUtil.concat(PInteger.INSTANCE.toBytes(7),PInteger.INSTANCE.toBytes(8)),
+ true)
+ }},
+ new int[]{4,4,4,4},
+ new int[]{1,1},
+ new Include(
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(2),
+ PInteger.INSTANCE.toBytes(3),
+ PInteger.INSTANCE.toBytes(6),
+ PInteger.INSTANCE.toBytes(7))),
+ new SeekNext(
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(2),
+ PInteger.INSTANCE.toBytes(3),
+ PInteger.INSTANCE.toBytes(7),
+ PInteger.INSTANCE.toBytes(9)),
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(2),
+ PInteger.INSTANCE.toBytes(4),
+ PInteger.INSTANCE.toBytes(5),
+ PInteger.INSTANCE.toBytes(6))),
+ new Finished(
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(3),
+ PInteger.INSTANCE.toBytes(4),
+ PInteger.INSTANCE.toBytes(7),
+ PInteger.INSTANCE.toBytes(9))))
+ );
+ testCases.addAll(
+ foreach(
+ new KeyRange[][]{
+ {
+ KeyRange.getKeyRange(
+ ByteUtil.concat(PInteger.INSTANCE.toBytes(1),PInteger.INSTANCE.toBytes(2)),
+ true,
+ ByteUtil.concat(PInteger.INSTANCE.toBytes(3),PInteger.INSTANCE.toBytes(4)),
+ true)
+ },
+ {
+ KeyRange.getKeyRange(PInteger.INSTANCE.toBytes(5)),
+ KeyRange.getKeyRange(PInteger.INSTANCE.toBytes(7))
+ },
+ {
+ PInteger.INSTANCE.getKeyRange(PInteger.INSTANCE.toBytes(9), true, PInteger.INSTANCE.toBytes(10), true)
+ }},
+ new int[]{4,4,4,4},
+ new int[]{1,0,0},
+ new Include(
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(1),
+ PInteger.INSTANCE.toBytes(3),
+ PInteger.INSTANCE.toBytes(5),
+ PInteger.INSTANCE.toBytes(9))),
+ new SeekNext(
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(2),
+ PInteger.INSTANCE.toBytes(3),
+ PInteger.INSTANCE.toBytes(7),
+ PInteger.INSTANCE.toBytes(11)),
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(2),
+ PInteger.INSTANCE.toBytes(4),
+ PInteger.INSTANCE.toBytes(5),
+ PInteger.INSTANCE.toBytes(9))),
+ new Finished(
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(3),
+ PInteger.INSTANCE.toBytes(4),
+ PInteger.INSTANCE.toBytes(7),
+ PInteger.INSTANCE.toBytes(11))))
+ );
+ testCases.addAll(
+ foreach(
+ new KeyRange[][]{
+ {
+ KeyRange.getKeyRange(
+ ByteUtil.concat(PInteger.INSTANCE.toBytes(1),PInteger.INSTANCE.toBytes(2)),
+ true,
+ ByteUtil.concat(PInteger.INSTANCE.toBytes(3),PInteger.INSTANCE.toBytes(4)),
+ true)
+ },
+ {
+ KeyRange.getKeyRange(ByteUtil.concat(PInteger.INSTANCE.toBytes(5),PInteger.INSTANCE.toBytes(6))),
+ KeyRange.getKeyRange(ByteUtil.concat(PInteger.INSTANCE.toBytes(7),PInteger.INSTANCE.toBytes(8)))
+ },
+ {
+ PInteger.INSTANCE.getKeyRange(PInteger.INSTANCE.toBytes(9), true, PInteger.INSTANCE.toBytes(10), true)
+ }},
+ new int[]{4,4,4,4,4},
+ new int[]{1,1,0},
+ new Include(
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(1),
+ PInteger.INSTANCE.toBytes(3),
+ PInteger.INSTANCE.toBytes(5),
+ PInteger.INSTANCE.toBytes(6),
+ PInteger.INSTANCE.toBytes(9))),
+ new SeekNext(
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(2),
+ PInteger.INSTANCE.toBytes(3),
+ PInteger.INSTANCE.toBytes(7),
+ PInteger.INSTANCE.toBytes(8),
+ PInteger.INSTANCE.toBytes(11)),
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(2),
+ PInteger.INSTANCE.toBytes(4),
+ PInteger.INSTANCE.toBytes(5),
+ PInteger.INSTANCE.toBytes(6),
+ PInteger.INSTANCE.toBytes(9))),
+ new Finished(
+ ByteUtil.concat(
+ PInteger.INSTANCE.toBytes(3),
+ PInteger.INSTANCE.toBytes(4),
+ PInteger.INSTANCE.toBytes(7),
+ PInteger.INSTANCE.toBytes(8),
+ PInteger.INSTANCE.toBytes(11))))
+ );
return testCases;
}
-
- private static Collection<?> foreach(KeyRange[][] ranges, int[] widths, Expectation... expectations) {
+
+ private static Collection<?> foreach(KeyRange[][] ranges, int[] widths, int[] slotSpans, Expectation... expectations) {
List<List<KeyRange>> cnf = Lists.transform(Lists.newArrayList(ranges), ARRAY_TO_LIST);
List<Object> ret = Lists.newArrayList();
- ret.add(new Object[] {cnf, widths, Arrays.asList(expectations)} );
+ ret.add(new Object[] {cnf, widths, slotSpans, Arrays.asList(expectations)} );
return ret;
}
@@ -439,6 +652,10 @@ public class SkipScanFilterTest extends TestCase {
this.rowkey = Bytes.toBytes(rowkey);
}
+ public Finished(byte[] rowkey) {
+ this.rowkey = rowkey;
+ }
+
@Override public void examine(SkipScanFilter skipper) throws IOException {
KeyValue kv = KeyValue.createFirstOnRow(rowkey);
skipper.reset();