SOLR-10237: Poly-Fields should work with subfield that have docValues=true

solr/CHANGES.txt

@@ -303,6 +303,8 @@ Bug Fixes
 * SOLR-10283: Learning to Rank (LTR) SolrFeature to reject searches with missing efi (External Feature Information) used by fq.
   (Christine Poerschke)
 
+* SOLR-10237: Poly-fields should work with subfields that have docValues=true (Tomás Fernández Löbbe, David Smiley)
+
 Optimizations
 ----------------------
 

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

@@ -76,20 +76,25 @@ public abstract class AbstractSubTypeFieldType extends FieldType implements Sche
    * and props of indexed=true, stored=false.
    *
    * @param schema the IndexSchema
-   * @param type   The {@link FieldType} of the prototype.
+   * @param subType   The {@link FieldType} of the prototype.
+   * @param polyField   The poly {@link FieldType}.
    * @return The {@link SchemaField}
    */
 
-  static SchemaField registerPolyFieldDynamicPrototype(IndexSchema schema, FieldType type) {
+  static SchemaField registerPolyFieldDynamicPrototype(IndexSchema schema, FieldType subType, FieldType polyField) {
-    String name = "*" + FieldType.POLY_FIELD_SEPARATOR + type.typeName;
+    String name = "*" + FieldType.POLY_FIELD_SEPARATOR + subType.typeName;
     Map<String, String> props = new HashMap<>();
     //Just set these, delegate everything else to the field type
     props.put("indexed", "true");
     props.put("stored", "false");
     props.put("multiValued", "false");
-    int p = SchemaField.calcProps(name, type, props);
+    // if polyField enables dv, add them to the subtypes
+    if (polyField.hasProperty(DOC_VALUES)) {
+      props.put("docValues", "true");
+    }
+    int p = SchemaField.calcProps(name, subType, props);
     SchemaField proto = SchemaField.create(name,
-            type, p, null);
+        subType, p, null);
     schema.registerDynamicFields(proto);
     return proto;
   }
@@ -107,7 +112,7 @@ public abstract class AbstractSubTypeFieldType extends FieldType implements Sche
     this.schema = schema;
     //Can't do this until here b/c the Dynamic Fields are not initialized until here.
     if (subType != null) {
-      SchemaField proto = registerPolyFieldDynamicPrototype(schema, subType);
+      SchemaField proto = registerPolyFieldDynamicPrototype(schema, subType, this);
       dynFieldProps = proto.getProperties();
     }
   }

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

@@ -75,10 +75,10 @@ public class LatLonType extends AbstractSubTypeFieldType implements SpatialQuery
       Point point = SpatialUtils.parsePointSolrException(externalVal, SpatialContext.GEO);
       //latitude
       SchemaField subLatSF = subField(field, LAT, schema);
-      f.add(subLatSF.createField(String.valueOf(point.getY())));
+      f.addAll(subLatSF.createFields(String.valueOf(point.getY())));
       //longitude
       SchemaField subLonSF = subField(field, LON, schema);
-      f.add(subLonSF.createField(String.valueOf(point.getX())));
+      f.addAll(subLonSF.createFields(String.valueOf(point.getX())));
     }
 
     if (field.stored()) {
@@ -86,6 +86,14 @@ public class LatLonType extends AbstractSubTypeFieldType implements SpatialQuery
     }
     return f;
   }
+  
+  @Override
+  protected void checkSupportsDocValues() {
+    // DocValues supported only when enabled at the fieldType 
+    if (!hasProperty(DOC_VALUES)) {
+      throw new UnsupportedOperationException("LatLonType can't have docValues=true in the field definition, use docValues=true in the fieldType definition, or in subFieldType/subFieldSuffix");
+    }
+  }
 
 
   @Override

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

@@ -71,12 +71,12 @@ public class PointType extends CoordinateFieldType implements SpatialQueryable {
     String[] point = parseCommaSeparatedList(externalVal, dimension);
 
     // TODO: this doesn't currently support polyFields as sub-field types
-    List<IndexableField> f = new ArrayList<>(dimension+1);
+    List<IndexableField> f = new ArrayList<>((dimension*2)+1);
 
     if (field.indexed()) {
       for (int i=0; i<dimension; i++) {
         SchemaField sf = subField(field, i, schema);
-        f.add(sf.createField(point[i]));
+        f.addAll(sf.createFields(point[i]));
       }
     }
 
@@ -84,7 +84,7 @@ public class PointType extends CoordinateFieldType implements SpatialQueryable {
       String storedVal = externalVal;  // normalize or not?
       f.add(createField(field.getName(), storedVal, StoredField.TYPE));
     }
-    
+
     return f;
   }
 
@@ -155,6 +155,14 @@ public class PointType extends CoordinateFieldType implements SpatialQueryable {
     }
     return bq.build();
   }
+  
+  @Override
+  protected void checkSupportsDocValues() {
+    // DocValues supported only when enabled at the fieldType 
+    if (!hasProperty(DOC_VALUES)) {
+      throw new UnsupportedOperationException("PointType can't have docValues=true in the field definition, use docValues=true in the fieldType definition, or in subFieldType/subFieldSuffix");
+    }
+  }
 
   /**
    * Calculates the range and creates a RangeQuery (bounding box) wrapped in a BooleanQuery (unless the dimension is

solr/core/src/test-files/solr/collection1/conf/schema.xml

@@ -392,15 +392,14 @@
   <fieldType name="uuid" class="solr.UUIDField"/>
 
   <!-- Try out some point types -->
-  <fieldType name="xy" class="solr.PointType" dimension="2" subFieldType="double"/>
+  <fieldType name="xy" class="solr.PointType" dimension="2" subFieldType="${solr.tests.doubleClass:pdouble}"  docValues="true"/>
-  <fieldType name="x" class="solr.PointType" dimension="1" subFieldType="double"/>
+  <fieldType name="x" class="solr.PointType" dimension="1" subFieldType="${solr.tests.doubleClass:pdouble}"/>
-  <fieldType name="tenD" class="solr.PointType" dimension="10" subFieldType="double"/>
+  <fieldType name="tenD" class="solr.PointType" dimension="10" subFieldType="${solr.tests.doubleClass:pdouble}"/>
-  <!-- Use the sub field suffix -->
+  <fieldType name="xyd" class="solr.PointType" dimension="2" subFieldSuffix="_d1"/>
-  <fieldType name="xyd" class="solr.PointType" dimension="2" subFieldSuffix="_d1_ndv"/>
   <fieldType name="geohash" class="solr.GeoHashField"/>
 
 
-  <fieldType name="latLon" class="solr.LatLonType" subFieldType="double"/>
+  <fieldType name="latLon" class="solr.LatLonType" subFieldType="${solr.tests.doubleClass:pdouble}"/>
 
   <!-- Currency type -->
   <fieldType name="currency" class="solr.CurrencyField" currencyConfig="currency.xml" multiValued="false"/>
@@ -621,7 +620,7 @@
   <dynamicField name="*_f1" type="${solr.tests.floatClass:pfloat}" indexed="true" stored="true" multiValued="false"/>
   <dynamicField name="*_d" type="${solr.tests.doubleClass:pdouble}" indexed="true" stored="true"/>
   <dynamicField name="*_d1" type="${solr.tests.doubleClass:pdouble}" indexed="true" stored="true" multiValued="false"/>
-  <dynamicField name="*_d1_ndv" type="${solr.tests.doubleClass:pdouble}" indexed="true" docValues="false" stored="true" multiValued="false"/>
+  <dynamicField name="*_d1_dv" type="${solr.tests.doubleClass:pdouble}" indexed="true" docValues="true" stored="true" multiValued="false"/>
   <dynamicField name="*_dt" type="${solr.tests.dateClass:pdate}" indexed="true" stored="true"/>
   <dynamicField name="*_dt1" type="${solr.tests.dateClass:pdate}" indexed="true" stored="true" multiValued="false"/>
 
@@ -671,7 +670,7 @@
   <dynamicField name="*_mfacet" type="string" indexed="true" stored="false" multiValued="true"/>
 
   <!-- Type used to index the lat and lon components for the "location" FieldType -->
-  <dynamicField name="*_coordinate" type="${solr.tests.doubleClass:pdouble}" indexed="true" stored="false" omitNorms="true"/>
+  <dynamicField name="*_coordinate" type="${solr.tests.doubleClass:pdouble}" indexed="true" stored="false" omitNorms="true" docValues="false"/>
 
   <dynamicField name="*_path" type="path" indexed="true" stored="true" omitNorms="true" multiValued="true"/>
   <dynamicField name="*_ancestor" type="ancestor_path" indexed="true" stored="true" omitNorms="true"

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

@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 package org.apache.solr.schema;
+import java.util.Arrays;
 import java.util.List;
 
 import org.apache.lucene.index.IndexableField;
@@ -45,11 +46,16 @@ public class PolyFieldTest extends SolrTestCaseJ4 {
     SchemaField home = schema.getField("home");
     assertNotNull(home);
     assertTrue(home.isPolyField());
+    
+    String subFieldType = "double";
+    if (usingPointFields()) {
+      subFieldType = "pdouble";
+    }
 
     SchemaField[] dynFields = schema.getDynamicFieldPrototypes();
     boolean seen = false;
     for (SchemaField dynField : dynFields) {
-      if (dynField.getName().equals("*" + FieldType.POLY_FIELD_SEPARATOR + "double")) {
+      if (dynField.getName().equals("*" + FieldType.POLY_FIELD_SEPARATOR + subFieldType)) {
         seen = true;
       }
     }
@@ -60,7 +66,7 @@ public class PolyFieldTest extends SolrTestCaseJ4 {
     assertNotNull(xy);
     assertTrue(xy instanceof PointType);
     assertTrue(xy.isPolyField());
-    home = schema.getFieldOrNull("home_0" + FieldType.POLY_FIELD_SEPARATOR + "double");
+    home = schema.getFieldOrNull("home_0" + FieldType.POLY_FIELD_SEPARATOR + subFieldType);
     assertNotNull(home);
     home = schema.getField("home");
     assertNotNull(home);
@@ -84,9 +90,14 @@ public class PolyFieldTest extends SolrTestCaseJ4 {
     double[] xy = new double[]{35.0, -79.34};
     String point = xy[0] + "," + xy[1];
     List<IndexableField> fields = home.createFields(point);
-    assertEquals(fields.size(), 3);//should be 3, we have a stored field
+    assertNotNull(pt.getSubType());
-    //first two fields contain the values, third is just stored and contains the original
+    int expectdNumFields = 3;//If DV=false, we expect one field per dimension plus a stored field
-    for (int i = 0; i < 3; i++) {
+    if (pt.subField(home, 0, schema).hasDocValues()) {
+      expectdNumFields+=2; // If docValues=true, then we expect two more fields
+    }
+    assertEquals("Unexpected fields created: " + Arrays.toString(fields.toArray()), expectdNumFields, fields.size());
+    //first two/four fields contain the values, last one is just stored and contains the original
+    for (int i = 0; i < expectdNumFields; i++) {
       boolean hasValue = fields.get(i).binaryValue() != null
           || fields.get(i).stringValue() != null
           || fields.get(i).numericValue() != null;
@@ -100,7 +111,7 @@ public class PolyFieldTest extends SolrTestCaseJ4 {
     home = schema.getField("home_ns");
     assertNotNull(home);
     fields = home.createFields(point);
-    assertEquals(fields.size(), 2);//should be 2, since we aren't storing
+    assertEquals(expectdNumFields - 1, fields.size(), 2);//one less field than with "home", since we aren't storing
 
     home = schema.getField("home_ns");
     assertNotNull(home);
@@ -111,17 +122,12 @@ public class PolyFieldTest extends SolrTestCaseJ4 {
       //
     }
 
-    
     SchemaField s1 = schema.getField("test_p");
     SchemaField s2 = schema.getField("test_p");
-    // If we use [Int/Double/Long/Float]PointField, we can't get the valueSource, since docValues is false
+    ValueSource v1 = s1.getType().getValueSource(s1, null);
-    if (s1.createFields("1,2").get(0).fieldType().pointDimensionCount() == 0) {
+    ValueSource v2 = s2.getType().getValueSource(s2, null);
-      assertFalse(s2.getType().isPointField());
+    assertEquals(v1, v2);
-      ValueSource v1 = s1.getType().getValueSource(s1, null);
+    assertEquals(v1.hashCode(), v2.hashCode());
-      ValueSource v2 = s2.getType().getValueSource(s2, null);
-      assertEquals(v1, v2);
-      assertEquals(v1.hashCode(), v2.hashCode());
-    }
   }
 
   @Test
@@ -181,5 +187,9 @@ public class PolyFieldTest extends SolrTestCaseJ4 {
     assertEquals(2, bq.clauses().size());
     clearIndex();
   }
+  
+  private boolean usingPointFields() {
+    return h.getCore().getLatestSchema().getField("foo_d1_dv").getType().isPointField();
+  }
 
 }

solr/core/src/test/org/apache/solr/update/DocumentBuilderTest.java

@@ -131,8 +131,8 @@ public class DocumentBuilderTest extends SolrTestCaseJ4 {
     doc.addField( "home", "2.2,3.3" );
     Document out = DocumentBuilder.toDocument( doc, core.getLatestSchema() );
     assertNotNull( out.get( "home" ) );//contains the stored value and term vector, if there is one
-    assertNotNull( out.getField( "home_0" + FieldType.POLY_FIELD_SEPARATOR + "double" ) );
+    assertNotNull( out.getField( "home_0" + FieldType.POLY_FIELD_SEPARATOR + System.getProperty("solr.tests.doubleClass", "pdouble") ) );
-    assertNotNull( out.getField( "home_1" + FieldType.POLY_FIELD_SEPARATOR + "double" ) );
+    assertNotNull( out.getField( "home_1" + FieldType.POLY_FIELD_SEPARATOR + System.getProperty("solr.tests.doubleClass", "pdouble") ) );
   }
   
   /**