SOLR-18379: remove DocsStreamer.KNOWN_TYPES for the ExternalizeStoredValuesAsObjects marker

changelog/unreleased/SOLR-18379-remove-docsstreamer-known-types.yml

@@ -0,0 +1,8 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: Remove the deprecated DocsStreamer.KNOWN_TYPES set in favour of the inherited FieldType.ExternalizeStoredValuesAsObjects marker; javabin responses now return stored values of quantized dense vector fields as numbers rather than strings, and stored values of custom BinaryField subclasses as binary rather than as their toExternal string.
+type: removed
+authors:
+  - name: Serhiy Bzhezytskyy
+links:
+  - name: SOLR-18379
+    url: https://issues.apache.org/jira/browse/SOLR-18379

solr/core/src/java/org/apache/solr/response/DocsStreamer.java

@@ -16,11 +16,8 @@
  */
 package org.apache.solr.response;
 
-import static org.apache.solr.schema.FieldType.ExternalizeStoredValuesAsObjects;
-
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
@@ -30,25 +27,9 @@
 import org.apache.solr.common.SolrDocument;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.response.transform.DocTransformer;
-import org.apache.solr.schema.BinaryField;
-import org.apache.solr.schema.BoolField;
-import org.apache.solr.schema.DatePointField;
-import org.apache.solr.schema.DenseVectorField;
-import org.apache.solr.schema.DoublePointField;
 import org.apache.solr.schema.FieldType;
-import org.apache.solr.schema.FloatPointField;
 import org.apache.solr.schema.IndexSchema;
-import org.apache.solr.schema.IntPointField;
-import org.apache.solr.schema.LongPointField;
 import org.apache.solr.schema.SchemaField;
-import org.apache.solr.schema.StrField;
-import org.apache.solr.schema.TextField;
-import org.apache.solr.schema.TrieDateField;
-import org.apache.solr.schema.TrieDoubleField;
-import org.apache.solr.schema.TrieField;
-import org.apache.solr.schema.TrieFloatField;
-import org.apache.solr.schema.TrieIntField;
-import org.apache.solr.schema.TrieLongField;
 import org.apache.solr.search.DocIterator;
 import org.apache.solr.search.DocList;
 import org.apache.solr.search.ReturnFields;
@@ -57,22 +38,6 @@
 
 /** This streams SolrDocuments from a DocList and applies transformer */
 public class DocsStreamer implements Iterator<SolrDocument> {
-  /**
-   * A hardcoded list of known Solr field types that will be trusted to control their own conversion
-   * of stored field values into external Objects (via {@link FieldType#toObject}) when returning
-   * {@link SolrDocument} instances to clients.
-   *
-   * <p>For historic reasons, this Set is consulted using an <em>equality</em> basis, so subclasses
-   * of these "known" types are not given the same level of trust.
-   *
-   * <p>Any field type not found in this list will have stored values externalized as
-   * <em>Strings</em> using {@link FieldType#toExternal} unless they implement {@link
-   * ExternalizeStoredValuesAsObjects}
-   *
-   * @deprecated new field types should not be added to this list, instead use {@link
-   *     ExternalizeStoredValuesAsObjects}
-   */
-  @Deprecated public static final Set<Class<? extends FieldType>> KNOWN_TYPES = new HashSet<>();
 
   private final ResultContext rctx;
   private final SolrDocumentFetcher docFetcher; // a collaborator of SolrIndexSearcher
@@ -217,41 +182,11 @@ public static Object getValue(SchemaField sf, IndexableField f) {
         return f.stringValue();
       }
     } else {
-      if (KNOWN_TYPES.contains(ft.getClass())
-          || ft instanceof FieldType.ExternalizeStoredValuesAsObjects) {
+      if (ft instanceof FieldType.ExternalizeStoredValuesAsObjects) {
         return ft.toObject(f);
       } else {
         return ft.toExternal(f);
       }
     }
   }
-
-  static {
-    // DO NOT ADD TO THIS SET ! ! ! !
-    // SEE JAVADOCS FOR KNOWN_TYPES !
-
-    KNOWN_TYPES.add(BoolField.class);
-    KNOWN_TYPES.add(StrField.class);
-    KNOWN_TYPES.add(TextField.class);
-    KNOWN_TYPES.add(TrieField.class);
-    KNOWN_TYPES.add(TrieIntField.class);
-    KNOWN_TYPES.add(TrieLongField.class);
-    KNOWN_TYPES.add(TrieFloatField.class);
-    KNOWN_TYPES.add(TrieDoubleField.class);
-    KNOWN_TYPES.add(TrieDateField.class);
-    KNOWN_TYPES.add(BinaryField.class);
-    KNOWN_TYPES.add(IntPointField.class);
-    KNOWN_TYPES.add(LongPointField.class);
-    KNOWN_TYPES.add(DoublePointField.class);
-    KNOWN_TYPES.add(FloatPointField.class);
-    // DenseVectorField extends FloatPointField but here we list DenseVectorField
-    // explicitly due to KNOWN_TYPES.contains use of the KNOWN_TYPES set
-    KNOWN_TYPES.add(DenseVectorField.class);
-    KNOWN_TYPES.add(DatePointField.class);
-    // We do not add UUIDField because UUID object is not a supported type in JavaBinCodec
-    // and if we write UUIDField.toObject, we wouldn't know how to handle it in the client side
-
-    // DO NOT ADD TO THIS SET ! ! ! !
-    // SEE JAVADOCS FOR KNOWN_TYPES !
-  }
 }

solr/core/src/java/org/apache/solr/schema/BinaryField.java

@@ -34,7 +34,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class BinaryField extends FieldType {
+public class BinaryField extends FieldType implements FieldType.ExternalizeStoredValuesAsObjects {
 
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 

solr/core/src/java/org/apache/solr/schema/BoolField.java

@@ -46,7 +46,8 @@
 import org.apache.solr.uninverting.UninvertingReader.Type;
 
 /** */
-public class BoolField extends PrimitiveFieldType {
+public class BoolField extends PrimitiveFieldType
+    implements FieldType.ExternalizeStoredValuesAsObjects {
   @Override
   public SortField getSortField(SchemaField field, boolean reverse) {
     field.checkSortability();

solr/core/src/java/org/apache/solr/schema/DatePointField.java

@@ -98,7 +98,8 @@
  *
  * @see PointField
  */
-public class DatePointField extends PointField implements DateValueFieldType {
+public class DatePointField extends PointField
+    implements DateValueFieldType, FieldType.ExternalizeStoredValuesAsObjects {
 
   public DatePointField() {
     type = NumberType.DATE;

solr/core/src/java/org/apache/solr/schema/DoublePointField.java

@@ -41,7 +41,8 @@
  * @see PointField
  * @see DoublePoint
  */
-public class DoublePointField extends PointField implements DoubleValueFieldType {
+public class DoublePointField extends PointField
+    implements DoubleValueFieldType, FieldType.ExternalizeStoredValuesAsObjects {
 
   public DoublePointField() {
     type = NumberType.DOUBLE;

solr/core/src/java/org/apache/solr/schema/FieldType.java

@@ -1506,12 +1506,16 @@ protected static Object unmarshalBase64SortValue(Object value) {
    * representation that will be returned to clients.
    *
    * <p>The default behavior if this interface is not implemented, is to delegate to {@link
-   * FieldType#toExternal(IndexableField)}, unless the field type is (exactly equal to) one of a
-   * specific list of {@link org.apache.solr.response.DocsStreamer#KNOWN_TYPES}
+   * FieldType#toExternal(IndexableField)}.
+   *
+   * <p>This interface is checked with {@code instanceof}, so subclasses of a field type that
+   * implements it are trusted as well. {@link UUIDField}, for example, inherits it from {@link
+   * StrField} and must keep returning a String from {@link FieldType#toObject(IndexableField)}: a
+   * {@code UUID} object is not a type {@code JavaBinCodec} can write, and clients would not know
+   * how to handle it.
    *
    * @see #toExternal
    * @see #toObject(IndexableField)
-   * @see org.apache.solr.response.DocsStreamer#KNOWN_TYPES
    */
   public static interface ExternalizeStoredValuesAsObjects {}
 

solr/core/src/java/org/apache/solr/schema/FloatPointField.java

@@ -41,7 +41,8 @@
  * @see PointField
  * @see FloatPoint
  */
-public class FloatPointField extends PointField implements FloatValueFieldType {
+public class FloatPointField extends PointField
+    implements FloatValueFieldType, FieldType.ExternalizeStoredValuesAsObjects {
 
   public FloatPointField() {
     type = NumberType.FLOAT;

solr/core/src/java/org/apache/solr/schema/IntPointField.java

@@ -39,7 +39,8 @@
  * @see PointField
  * @see IntPoint
  */
-public class IntPointField extends PointField implements IntValueFieldType {
+public class IntPointField extends PointField
+    implements IntValueFieldType, FieldType.ExternalizeStoredValuesAsObjects {
 
   public IntPointField() {
     type = NumberType.INTEGER;

solr/core/src/java/org/apache/solr/schema/LongPointField.java

@@ -39,7 +39,8 @@
  * @see PointField
  * @see LongPoint
  */
-public class LongPointField extends PointField implements LongValueFieldType {
+public class LongPointField extends PointField
+    implements LongValueFieldType, FieldType.ExternalizeStoredValuesAsObjects {
 
   public LongPointField() {
     type = NumberType.LONG;

solr/core/src/java/org/apache/solr/schema/StrField.java

@@ -34,7 +34,8 @@
 import org.apache.solr.search.QParser;
 import org.apache.solr.uninverting.UninvertingReader.Type;
 
-public class StrField extends PrimitiveFieldType {
+public class StrField extends PrimitiveFieldType
+    implements FieldType.ExternalizeStoredValuesAsObjects {
 
   @Override
   protected void init(IndexSchema schema, Map<String, String> args) {

solr/core/src/java/org/apache/solr/schema/TextField.java

@@ -44,7 +44,7 @@
  * <code>TextField</code> is the basic type for configurable text analysis. Analyzers for field
  * types using this implementation should be defined in the schema.
  */
-public class TextField extends FieldType {
+public class TextField extends FieldType implements FieldType.ExternalizeStoredValuesAsObjects {
   protected boolean autoGeneratePhraseQueries;
   protected boolean enableGraphQueries;
   protected SolrQueryParserBase.SynonymQueryStyle synonymQueryStyle;

solr/core/src/java/org/apache/solr/schema/TrieField.java

@@ -88,7 +88,8 @@
  * @see PointField
  */
 @Deprecated
-public class TrieField extends NumericFieldType {
+public class TrieField extends NumericFieldType
+    implements FieldType.ExternalizeStoredValuesAsObjects {
   public static final int DEFAULT_PRECISION_STEP = 8;
 
   protected int precisionStepArg =

solr/core/src/test/org/apache/solr/response/DocsStreamerTest.java

@@ -25,20 +25,64 @@
 
 public class DocsStreamerTest extends SolrTestCaseJ4 {
 
+  private static final List<Float> VECTOR = Arrays.asList(1.1f, 2.2f, 3.3f, 4.4f);
+
   @BeforeClass
   public static void beforeClass() throws Exception {
     System.setProperty(
         "solr.index.updatelog.enabled", "false"); // schema12 doesn't support _version_
-    initCore("solrconfig.xml", "schema12.xml");
   }
 
+  // Each test method initializes its own core, because the schemas holding the quantized
+  // field types are not the same file as the one holding the plain DenseVectorField.
   public void testDenseVectorField() throws Exception {
-    List<Float> values = Arrays.asList(1.1f, 2.2f, 3.3f, 4.4f);
-    SchemaField sf = h.getCore().getLatestSchema().getField("vector");
-    List<IndexableField> fields = sf.createFields(values);
+    try {
+      initCore("solrconfig.xml", "schema12.xml");
+      assertStoredValues("vector", VECTOR);
+    } finally {
+      deleteCore();
+    }
+  }
+
+  // ScalarQuantizedDenseVectorField is a subclass of DenseVectorField, and the
+  // ExternalizeStoredValuesAsObjects marker is consulted with instanceof, so the subclass takes
+  // the same toObject path as its superclass: its stored values are externalized as Floats.
+  // This pins that agreement so it cannot change without a test failing.
+  public void testScalarQuantizedDenseVectorField() throws Exception {
+    try {
+      initCore("solrconfig_codec.xml", "schema-densevector-quantized.xml");
+      // plain DenseVectorField, marker inherited from FloatPointField: Float objects
+      assertStoredValues("vector", VECTOR);
+      // subclass of it, so it inherits the marker too: Float objects as well
+      assertStoredValues("v_scalar_default", VECTOR);
+    } finally {
+      deleteCore();
+    }
+  }
+
+  // Same for the other DenseVectorField subclass.
+  public void testBinaryQuantizedDenseVectorField() throws Exception {
+    try {
+      initCore("solrconfig-basic.xml", "schema-densevector-bq.xml");
+      assertStoredValues("v_bq", VECTOR);
+    } finally {
+      deleteCore();
+    }
+  }
+
+  // Asserts what DocsStreamer.getValue returns for every stored field that fieldName creates
+  // for VECTOR. The first created field is the indexed one, so it is skipped.
+  private void assertStoredValues(String fieldName, List<?> expected) {
+    SchemaField sf = h.getCore().getLatestSchema().getField(fieldName);
+    List<IndexableField> fields = sf.createFields(VECTOR);
+    assertEquals(fieldName + " created field count", expected.size() + 1, fields.size());
     for (int idx = 1; idx < fields.size(); ++idx) {
       Object value = DocsStreamer.getValue(sf, fields.get(idx));
-      assertEquals(values.get(idx - 1), value);
+      Object want = expected.get(idx - 1);
+      String label = fieldName + " element " + (idx - 1);
+      assertNotNull(label, value);
+      assertEquals(label + " type", want.getClass(), value.getClass());
+      assertEquals(label, want, value);
     }
   }
 }

solr/core/src/test/org/apache/solr/schema/TestBinaryField.java

@@ -137,7 +137,7 @@ public void testSimple() throws Exception {
         assertArrayEquals(expected_bytes, (byte[]) d.getFieldValue("rev_data"));
         assertArrayEquals(expected_bytes, (byte[]) d.getFieldValue("rev_data_dv"));
 
-        assertEquals(expected_string, d.getFieldValue("str_data"));
+        assertArrayEquals(expected_bytes, (byte[]) d.getFieldValue("str_data"));
         assertEquals(expected_string, d.getFieldValue("str_data_dv"));
       }
       for (Bean d : beans) {
@@ -151,7 +151,7 @@ public void testSimple() throws Exception {
         assertArrayEquals(expected_bytes, d.rev_data);
         assertArrayEquals(expected_bytes, d.rev_data_dv);
 
-        assertEquals(expected_string, d.str_data);
+        assertArrayEquals(expected_bytes, d.str_data);
         assertEquals(expected_string, d.str_data_dv);
       }
     }
@@ -170,7 +170,7 @@ public static class Bean {
     @Field byte[] data_dv;
     @Field byte[] rev_data;
     @Field byte[] rev_data_dv;
-    @Field String str_data;
+    @Field byte[] str_data;
     @Field String str_data_dv;
   }
 }