PHOENIX-3705 SkipScanFilter may repeatedly copy rowKey Columns to sta…

…rtKey

phoenix-core/src/main/java/org/apache/phoenix/filter/SkipScanFilter.java

@@ -445,13 +445,26 @@ private ReturnCode navigate(final byte[] currentKey, final int offset, final int
                     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);

phoenix-core/src/main/java/org/apache/phoenix/schema/RowKeySchema.java

@@ -81,19 +81,25 @@ public int getMaxFields() {
     }
 
     // "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);