[GitHub] [incubator-pinot] yupeng9 commented on a change in pull request #6176: Merge common APIs for Dictionary

[GitBox <gi...@apache.org>]

yupeng9 commented on a change in pull request #6176:
URL: https://github.com/apache/incubator-pinot/pull/6176#discussion_r510404302



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/BaseImmutableDictionary.java
##########
@@ -81,6 +80,31 @@ public int indexOf(String stringValue) {
     return (index >= 0) ? index : NULL_VALUE_INDEX;
   }
 
+  @Override
+  public IntSet getDictIdsInRange(String lower, String upper, boolean includeLower, boolean includeUpper) {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public int compare(int dictId1, int dictId2) {
+    return Integer.compare(dictId1, dictId2);
+  }
+
+  @Override
+  public Comparable getMinVal() {
+    return (Comparable) get(0);
+  }
+
+  @Override
+  public Comparable getMaxVal() {
+    return (Comparable) get(_length - 1);
+  }
+
+  @Override
+  public Object getSortedValues() {
+    throw new UnsupportedOperationException();

Review comment:
       why is this unsupported? since it returns true for `isSorted `

##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/BaseImmutableDictionary.java
##########
@@ -81,6 +80,31 @@ public int indexOf(String stringValue) {
     return (index >= 0) ? index : NULL_VALUE_INDEX;
   }
 
+  @Override
+  public IntSet getDictIdsInRange(String lower, String upper, boolean includeLower, boolean includeUpper) {
+    throw new UnsupportedOperationException();

Review comment:
       why can this be supported?

##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/BaseImmutableDictionary.java
##########
@@ -81,6 +80,31 @@ public int indexOf(String stringValue) {
     return (index >= 0) ? index : NULL_VALUE_INDEX;
   }
 
+  @Override
+  public IntSet getDictIdsInRange(String lower, String upper, boolean includeLower, boolean includeUpper) {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public int compare(int dictId1, int dictId2) {
+    return Integer.compare(dictId1, dictId2);
+  }
+
+  @Override
+  public Comparable getMinVal() {
+    return (Comparable) get(0);
+  }
+
+  @Override
+  public Comparable getMaxVal() {
+    return (Comparable) get(_length - 1);
+  }
+
+  @Override
+  public Object getSortedValues() {
+    throw new UnsupportedOperationException();

Review comment:
       If you want a default impl, then perhaps just remove it, so the subclass must implement it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org