jpountz commented on a change in pull request #443:
URL: https://github.com/apache/lucene/pull/443#discussion_r751361476



##########
File path: lucene/facet/src/java/org/apache/lucene/facet/FacetUtils.java
##########
@@ -81,4 +82,17 @@ public long cost() {
       }
     };
   }
+
+  /**
+   * Determine whether-or-not an index segment is using the older-style binary 
format or the newer
+   * NumericDocValues format for storing taxonomy faceting ordinals (for the 
specified field).
+   *
+   * @deprecated Please do not rely on this method. It is added as a temporary 
measure for providing
+   *     index backwards-compatibility with Lucene 8 and earlier indexes, and 
will be removed in
+   *     Lucene 10.
+   */
+  @Deprecated
+  public static boolean usesOlderBinaryOrdinals(LeafReader reader) {
+    return reader.getMetaData().getCreatedVersionMajor() <= 8;
+  }

Review comment:
       instead of just being a flag, maybe this could abstract reading ordinals 
from the index?
   
   ```java
     public static SortedNumericDocValues getOrdinals(LeafReader reader, String 
field) {
   ```

##########
File path: 
lucene/facet/src/java/org/apache/lucene/facet/taxonomy/DocValuesOrdinalsReader.java
##########
@@ -41,13 +48,21 @@ public DocValuesOrdinalsReader(String field) {
 
   @Override
   public OrdinalsSegmentReader getReader(LeafReaderContext context) throws 
IOException {
-    BinaryDocValues values0 = context.reader().getBinaryDocValues(field);
-    if (values0 == null) {
-      values0 = DocValues.emptyBinary();
+    final SortedNumericDocValues dv;
+    if (FacetUtils.usesOlderBinaryOrdinals(context.reader())) {
+      // Wrap the binary values so they appear as numeric doc values. Delegate 
to the #decode method
+      // so sub-class decoding implementations are honored:
+      SortedNumericDocValues wrapped =
+          BackCompatSortedNumericDocValues.wrap(
+              context.reader().getBinaryDocValues(field), this::decode);

Review comment:
       I'm confused: this is the only call site that takes a decoder, and yet 
it uses the same decoding logic as the default logic. Do we need to expose a 
decoding function at all?




-- 
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.

To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to