HBASE-19027 Honor the CellComparator of ScanInfo in scanning over a store

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java

@@ -47,6 +47,7 @@ import org.apache.hadoop.hbase.regionserver.handler.ParallelSeekHandler;
 import org.apache.hadoop.hbase.regionserver.querymatcher.CompactionScanQueryMatcher;
 import org.apache.hadoop.hbase.regionserver.querymatcher.ScanQueryMatcher;
 import org.apache.hadoop.hbase.regionserver.querymatcher.UserScanQueryMatcher;
+import org.apache.hadoop.hbase.shaded.com.google.common.base.Preconditions;
 import org.apache.hadoop.hbase.util.CollectionUtils;
 import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
 import org.apache.yetus.audience.InterfaceAudience;
@@ -67,6 +68,7 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
   private static final Log LOG = LogFactory.getLog(StoreScanner.class);
   // In unit tests, the store could be null
   protected final HStore store;
+  private final CellComparator comparator;
   private ScanQueryMatcher matcher;
   protected KeyValueHeap heap;
   private boolean cacheBlocks;
@@ -164,6 +166,7 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
     this.readPt = readPt;
     this.store = store;
     this.cacheBlocks = cacheBlocks;
+    this.comparator = Preconditions.checkNotNull(scanInfo.getComparator());
     get = scan.isGetScan();
     explicitColumnQuery = numColumns > 0;
     this.scan = scan;
@@ -252,7 +255,7 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
       this.storeOffset = scan.getRowOffsetPerColumnFamily();
       addCurrentScanners(scanners);
       // Combine all seeked scanners with a heap
-      resetKVHeap(scanners, store.getComparator());
+      resetKVHeap(scanners, comparator);
     } catch (IOException e) {
       // remove us from the HStore#changedReaderObservers here or we'll have no chance to
       // and might cause memory leak
@@ -311,7 +314,7 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
     seekScanners(scanners, matcher.getStartKey(), false, parallelSeekEnabled);
     addCurrentScanners(scanners);
     // Combine all seeked scanners with a heap
-    resetKVHeap(scanners, store.getComparator());
+    resetKVHeap(scanners, comparator);
   }
 
   private void seekAllScanner(ScanInfo scanInfo, List<? extends KeyValueScanner> scanners)
@@ -319,7 +322,7 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
     // Seek all scanners to the initial key
     seekScanners(scanners, matcher.getStartKey(), false, parallelSeekEnabled);
     addCurrentScanners(scanners);
-    resetKVHeap(scanners, scanInfo.getComparator());
+    resetKVHeap(scanners, comparator);
   }
 
   // For mob compaction only as we do not have a Store instance when doing mob compaction.
@@ -543,9 +546,6 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
       scannerContext.clearProgress();
     }
 
-    // Only do a sanity-check if store and comparator are available.
-    CellComparator comparator = store != null ? store.getComparator() : null;
-
     int count = 0;
     long totalBytesRead = 0;
 
@@ -997,7 +997,7 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
       newCurrentScanners = new ArrayList<>(fileScanners.size() + memstoreScanners.size());
       newCurrentScanners.addAll(fileScanners);
       newCurrentScanners.addAll(memstoreScanners);
-      newHeap = new KeyValueHeap(newCurrentScanners, store.getComparator());
+      newHeap = new KeyValueHeap(newCurrentScanners, comparator);
     } catch (Exception e) {
       LOG.warn("failed to switch to stream read", e);
       if (fileScanners != null) {

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/ScanQueryMatcher.java

@@ -402,7 +402,7 @@ public abstract class ScanQueryMatcher implements ShipperListener {
       columnTracker = (NewVersionBehaviorTracker) deleteTracker;
     } else if (columns == null || columns.size() == 0) {
       columnTracker = new ScanWildcardColumnTracker(scanInfo.getMinVersions(), maxVersionToCheck,
-          oldestUnexpiredTS);
+          oldestUnexpiredTS, scanInfo.getComparator());
     } else {
       columnTracker = new ExplicitColumnTracker(columns, scanInfo.getMinVersions(),
         maxVersionToCheck, oldestUnexpiredTS);

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/ScanWildcardColumnTracker.java

@@ -21,7 +21,7 @@ package org.apache.hadoop.hbase.regionserver.querymatcher;
 import java.io.IOException;
 
 import org.apache.hadoop.hbase.Cell;
-import org.apache.hadoop.hbase.CellComparatorImpl;
+import org.apache.hadoop.hbase.CellComparator;
 import org.apache.hadoop.hbase.CellUtil;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.PrivateCellUtil;
@@ -49,16 +49,20 @@ public class ScanWildcardColumnTracker implements ColumnTracker {
 
   private long oldestStamp;
 
+  private final CellComparator comparator;
   /**
    * Return maxVersions of every row.
    * @param minVersion Minimum number of versions to keep
    * @param maxVersion Maximum number of versions to return
    * @param oldestUnexpiredTS oldest timestamp that has not expired according to the TTL.
+   * @param comparator used to compare the qualifier of cell
    */
-  public ScanWildcardColumnTracker(int minVersion, int maxVersion, long oldestUnexpiredTS) {
+  public ScanWildcardColumnTracker(int minVersion, int maxVersion,
+      long oldestUnexpiredTS, CellComparator comparator) {
     this.maxVersions = maxVersion;
     this.minVersions = minVersion;
     this.oldestStamp = oldestUnexpiredTS;
+    this.comparator = comparator;
   }
 
   /**
@@ -85,7 +89,7 @@ public class ScanWildcardColumnTracker implements ColumnTracker {
       // do not count a delete marker as another version
       return checkVersion(type, timestamp);
     }
-    int cmp = CellComparatorImpl.COMPARATOR.compareQualifiers(cell, this.columnCell);
+    int cmp = comparator.compareQualifiers(cell, this.columnCell);
     if (cmp == 0) {
       if (ignoreCount) {
         return ScanQueryMatcher.MatchCode.INCLUDE;

hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java

@@ -29,7 +29,6 @@ import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.hadoop.hbase.regionserver.querymatcher.ScanDeleteTracker;
 import org.apache.hadoop.hbase.Cell;
 import org.apache.hadoop.hbase.CellComparator;
-import org.apache.hadoop.hbase.CellComparatorImpl;
 import org.apache.hadoop.hbase.CellUtil;
 import org.apache.hadoop.hbase.KeyValue;
 import org.apache.hadoop.hbase.KeyValue.Type;
@@ -64,7 +63,7 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker {
   // Need to track it per ts.
   private List<Triple<List<Tag>, Byte, Long>> visibilityTagsDeleteFamilyVersion = new ArrayList<>();
   private List<Pair<List<Tag>, Byte>> visibilityTagsDeleteColumns;
-  // Tracking as List<List> is to handle same ts cell but different visibility tag. 
+  // Tracking as List<List> is to handle same ts cell but different visibility tag.
   // TODO : Need to handle puts with same ts but different vis tags.
   private List<Pair<List<Tag>, Byte>> visiblityTagsDeleteColumnVersion = new ArrayList<>();
 
@@ -247,7 +246,7 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker {
         }
       }
       if (deleteCell != null) {
-        int ret = CellComparatorImpl.COMPARATOR.compareQualifiers(cell, deleteCell);
+        int ret = comparator.compareQualifiers(cell, deleteCell);
         if (ret == 0) {
           if (deleteType == KeyValue.Type.DeleteColumn.getCode()) {
             if (visibilityTagsDeleteColumns != null) {

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/querymatcher/TestScanWildcardColumnTracker.java

@@ -26,6 +26,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.hadoop.hbase.CellComparatorImpl;
 import org.apache.hadoop.hbase.KeyValue;
 import org.apache.hadoop.hbase.regionserver.querymatcher.ScanQueryMatcher.MatchCode;
 import org.apache.hadoop.hbase.testclassification.RegionServerTests;
@@ -41,7 +42,8 @@ public class TestScanWildcardColumnTracker {
 
   @Test
   public void testCheckColumnOk() throws IOException {
-    ScanWildcardColumnTracker tracker = new ScanWildcardColumnTracker(0, VERSIONS, Long.MIN_VALUE);
+    ScanWildcardColumnTracker tracker = new ScanWildcardColumnTracker(
+        0, VERSIONS, Long.MIN_VALUE, CellComparatorImpl.COMPARATOR);
 
     // Create list of qualifiers
     List<byte[]> qualifiers = new ArrayList<>(4);
@@ -73,7 +75,8 @@ public class TestScanWildcardColumnTracker {
 
   @Test
   public void testCheckColumnEnforceVersions() throws IOException {
-    ScanWildcardColumnTracker tracker = new ScanWildcardColumnTracker(0, VERSIONS, Long.MIN_VALUE);
+    ScanWildcardColumnTracker tracker = new ScanWildcardColumnTracker(
+        0, VERSIONS, Long.MIN_VALUE, CellComparatorImpl.COMPARATOR);
 
     // Create list of qualifiers
     List<byte[]> qualifiers = new ArrayList<>(4);
@@ -106,7 +109,8 @@ public class TestScanWildcardColumnTracker {
 
   @Test
   public void DisabledTestCheckColumnWrongOrder() {
-    ScanWildcardColumnTracker tracker = new ScanWildcardColumnTracker(0, VERSIONS, Long.MIN_VALUE);
+    ScanWildcardColumnTracker tracker = new ScanWildcardColumnTracker(
+        0, VERSIONS, Long.MIN_VALUE, CellComparatorImpl.COMPARATOR);
 
     // Create list of qualifiers
     List<byte[]> qualifiers = new ArrayList<>(2);