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/calcite Commit: c8612fa1b09f883726102951626798244f73db17 Parents: cf65fb2 Author: chenglei <cheng...@apache.org> Authored: Sat Mar 4 10:45:57 2017 +0800 Committer: chenglei <cheng...@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();