SOLR-10255 Add support for docValues to solr.BinaryField (#2536)

serba · Alexey Serba · web-flow · commit 6ab6c4abf21d · 2024-07-17T15:47:10.000-04:00
Co-authored-by: Alexey Serba &lt;alexeys@zillowgroup.com&gt;
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
@@ -122,6 +122,8 @@ New Features
   overridden with a property ('solr.query.minPrefixLength').  Users may also override their collection-wide setting for individual queries by providing a
   `minPrefixQueryTermLength` local-param.  (Jason Gerlowski, David Smiley)
 
+* SOLR-10255: Add support for docValues to solr.BinaryField. (Alexey Serba via Mikhail Khludnev, David Smiley)
+
 Improvements
 ---------------------
 * SOLR-17137: Enable Prometheus exporter to communicate with SSL protected Solr. (Eivind Bergstøl via Eric Pugh)
diff --git a/solr/core/src/java/org/apache/solr/schema/BinaryField.java b/solr/core/src/java/org/apache/solr/schema/BinaryField.java
@@ -20,7 +20,11 @@
 import java.lang.invoke.MethodHandles;
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
 import java.util.Base64;
+import java.util.Collections;
+import java.util.List;
+import org.apache.lucene.document.BinaryDocValuesField;
 import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.search.SortField;
 import org.apache.lucene.util.BytesRef;
@@ -42,6 +46,11 @@ public void checkSchemaField(SchemaField field) {
           SolrException.ErrorCode.SERVER_ERROR,
           "Field type " + this + " is 'large'; not supported (yet)");
     }
+    if (field.hasDocValues() && field.multiValued()) {
+      throw new SolrException(
+          SolrException.ErrorCode.SERVER_ERROR,
+          "Field type " + this + " does not support multiple doc values");
+    }
   }
 
   private String toBase64String(ByteBuffer buf) {
@@ -94,6 +103,10 @@ public IndexableField createField(SchemaField field, Object val) {
       log.trace("Ignoring unstored binary field: {}", field);
       return null;
     }
+    return new org.apache.lucene.document.StoredField(field.getName(), getBytesRef(val));
+  }
+
+  private static BytesRef getBytesRef(Object val) {
     byte[] buf = null;
     int offset = 0, len = 0;
     if (val instanceof byte[]) {
@@ -112,7 +125,31 @@ public IndexableField createField(SchemaField field, Object val) {
       len = buf.length;
     }
 
-    return new org.apache.lucene.document.StoredField(field.getName(), buf, offset, len);
+    return new BytesRef(buf, offset, len);
+  }
+
+  @Override
+  public List<IndexableField> createFields(SchemaField field, Object val) {
+    IndexableField fval = createField(field, val);
+
+    if (field.hasDocValues() && !field.multiValued()) {
+      IndexableField docval = new BinaryDocValuesField(field.getName(), getBytesRef(val));
+
+      // Only create list if we have 2 values...
+      if (fval != null) {
+        List<IndexableField> fields = new ArrayList<>(2);
+        fields.add(fval);
+        fields.add(docval);
+        return fields;
+      }
+
+      fval = docval;
+    }
+    return Collections.singletonList(fval);
+  }
+
+  @Override
+  protected void checkSupportsDocValues() { // we support DocValues
   }
 
   @Override
diff --git a/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java b/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java
@@ -639,7 +639,7 @@ private Object decodeDVField(
       case BINARY:
         BinaryDocValues bdv = e.getBinaryDocValues(localId, leafReader, readerOrd);
         if (bdv != null) {
-          return BytesRef.deepCopyOf(bdv.binaryValue());
+          return BytesRef.deepCopyOf(bdv.binaryValue()).bytes;
         }
         return null;
       case SORTED:
diff --git a/solr/core/src/test-files/solr/collection1/conf/bad-schema-unsupported-docValues.xml b/solr/core/src/test-files/solr/collection1/conf/bad-schema-unsupported-docValues.xml
@@ -17,10 +17,10 @@
 -->
 
 <schema name="bad-schema-docValues-unsupported" version="1.6">
-  <fieldType name="binary" class="solr.BinaryField"/>
+  <fieldType name="random" class="solr.RandomSortField" />
 
 
-  <!-- change the type if BinaryField gets doc values -->
-  <field name="id" type="binary" docValues="true"/>
+  <!-- change the type if RandomSortField gets doc values -->
+  <field name="id" type="random" docValues="true"/>
 
 </schema>
diff --git a/solr/core/src/test-files/solr/collection1/conf/schema-binaryfield.xml b/solr/core/src/test-files/solr/collection1/conf/schema-binaryfield.xml
@@ -33,6 +33,7 @@
 
   <field name="id" type="string" indexed="true" stored="true" multiValued="false" required="true"/>
   <field name="data" type="binary" stored="true"/>
+  <field name="data_dv" type="binary" stored="false" docValues="true" />
 
 
   <uniqueKey>id</uniqueKey>
diff --git a/solr/core/src/test/org/apache/solr/schema/TestBinaryField.java b/solr/core/src/test/org/apache/solr/schema/TestBinaryField.java
@@ -82,73 +82,78 @@ public void testSimple() throws Exception {
       doc = new SolrInputDocument();
       doc.addField("id", 1);
       doc.addField("data", ByteBuffer.wrap(buf, 2, 5));
+      doc.addField("data_dv", ByteBuffer.wrap(buf, 2, 5));
       client.add(doc);
 
       doc = new SolrInputDocument();
       doc.addField("id", 2);
       doc.addField("data", ByteBuffer.wrap(buf, 4, 3));
+      doc.addField("data_dv", ByteBuffer.wrap(buf, 4, 3));
       client.add(doc);
 
       doc = new SolrInputDocument();
       doc.addField("id", 3);
       doc.addField("data", buf);
+      doc.addField("data_dv", buf);
       client.add(doc);
 
       client.commit();
 
-      QueryResponse resp = client.query(new SolrQuery("*:*"));
+      QueryResponse resp = client.query(new SolrQuery("*:*").setFields("id", "data", "data_dv"));
       SolrDocumentList res = resp.getResults();
       List<Bean> beans = resp.getBeans(Bean.class);
       assertEquals(3, res.size());
       assertEquals(3, beans.size());
       for (SolrDocument d : res) {
-
         Integer id = Integer.parseInt(d.getFieldValue("id").toString());
-        byte[] data = (byte[]) d.getFieldValue("data");
-        if (id == 1) {
-          assertEquals(5, data.length);
-          for (int i = 0; i < data.length; i++) {
-            byte b = data[i];
-            assertEquals((byte) (i + 2), b);
-          }
-
-        } else if (id == 2) {
-          assertEquals(3, data.length);
-          for (int i = 0; i < data.length; i++) {
-            byte b = data[i];
-            assertEquals((byte) (i + 4), b);
-          }
-
-        } else if (id == 3) {
-          assertEquals(10, data.length);
-          for (int i = 0; i < data.length; i++) {
-            byte b = data[i];
-            assertEquals((byte) i, b);
+        for (String field : new String[] {"data", "data_dv"}) {
+          byte[] data = (byte[]) d.getFieldValue(field);
+          if (id == 1) {
+            assertEquals(5, data.length);
+            for (int i = 0; i < data.length; i++) {
+              byte b = data[i];
+              assertEquals((byte) (i + 2), b);
+            }
+
+          } else if (id == 2) {
+            assertEquals(3, data.length);
+            for (int i = 0; i < data.length; i++) {
+              byte b = data[i];
+              assertEquals((byte) (i + 4), b);
+            }
+
+          } else if (id == 3) {
+            assertEquals(10, data.length);
+            for (int i = 0; i < data.length; i++) {
+              byte b = data[i];
+              assertEquals((byte) i, b);
+            }
           }
         }
       }
       for (Bean d : beans) {
         Integer id = Integer.parseInt(d.id);
-        byte[] data = d.data;
-        if (id == 1) {
-          assertEquals(5, data.length);
-          for (int i = 0; i < data.length; i++) {
-            byte b = data[i];
-            assertEquals((byte) (i + 2), b);
-          }
-
-        } else if (id == 2) {
-          assertEquals(3, data.length);
-          for (int i = 0; i < data.length; i++) {
-            byte b = data[i];
-            assertEquals((byte) (i + 4), b);
-          }
-
-        } else if (id == 3) {
-          assertEquals(10, data.length);
-          for (int i = 0; i < data.length; i++) {
-            byte b = data[i];
-            assertEquals((byte) i, b);
+        for (byte[] data : new byte[][] {d.data, d.data_dv}) {
+          if (id == 1) {
+            assertEquals(5, data.length);
+            for (int i = 0; i < data.length; i++) {
+              byte b = data[i];
+              assertEquals((byte) (i + 2), b);
+            }
+
+          } else if (id == 2) {
+            assertEquals(3, data.length);
+            for (int i = 0; i < data.length; i++) {
+              byte b = data[i];
+              assertEquals((byte) (i + 4), b);
+            }
+
+          } else if (id == 3) {
+            assertEquals(10, data.length);
+            for (int i = 0; i < data.length; i++) {
+              byte b = data[i];
+              assertEquals((byte) i, b);
+            }
           }
         }
       }
@@ -158,5 +163,6 @@ public void testSimple() throws Exception {
   public static class Bean {
     @Field String id;
     @Field byte[] data;
+    @Field byte[] data_dv;
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -639,7 +639,7 @@ private Object decodeDVField(`
`639`	`639`	`case BINARY:`
`640`	`640`	`BinaryDocValues bdv = e.getBinaryDocValues(localId, leafReader, readerOrd);`
`641`	`641`	`if (bdv != null) {`
`642`		`- return BytesRef.deepCopyOf(bdv.binaryValue());`
	`642`	`+ return BytesRef.deepCopyOf(bdv.binaryValue()).bytes;`
`643`	`643`	`}`
`644`	`644`	`return null;`
`645`	`645`	`case SORTED:`