SOLR-10846: ExternalFileField/FloatFieldSource should throw a clear exception on initialization with a Points-based keyField, which is not supported

solr/CHANGES.txt

@@ -569,6 +569,9 @@ Other Changes
 
 * SOLR-10926: Increase the odds of randomly choosing point fields in our SolrTestCaseJ4 numeric type randomization.
   (hossman, Steve Rowe)
+  
+* SOLR-10846: ExternalFileField/FloatFieldSource should throw a clear exception on initialization with
+  a Points-based keyField, which is not supported. (hossman, Steve Rowe)
 
 ==================  6.7.0 ==================
 

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

@@ -22,6 +22,7 @@ import java.util.Map;
 import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.queries.function.ValueSource;
 import org.apache.lucene.search.SortField;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.response.TextResponseWriter;
 import org.apache.solr.search.QParser;
 import org.apache.solr.search.function.FileFloatSource;
@@ -122,5 +123,10 @@ public class ExternalFileField extends FieldType implements SchemaAware {
   @Override
   public void inform(IndexSchema schema) {
     this.schema = schema;
+    
+    if (keyFieldName != null && schema.getFieldType(keyFieldName).isPointField()) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+          "keyField '" + keyFieldName + "' has a Point field type, which is not supported.");
+    }
   }
 }

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

@@ -0,0 +1,44 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+  -->
+
+<schema name="example" version="1.6">
+
+
+  <field name="id" type="string" indexed="true" stored="true" required="true" multiValued="false"/>
+  <field name="eff" type="eff"/>
+
+
+  <!-- Field to use to determine and enforce document uniqueness. 
+       Unless this field is marked with required="false", it will be a required field
+    -->
+  <uniqueKey>id</uniqueKey>
+
+
+  <!-- The StrField type is not analyzed, but indexed/stored verbatim. -->
+  <fieldType name="string" class="solr.StrField" sortMissingLast="true"/>
+  
+  <fieldType name="pint" class="solr.IntPointField"/>
+  <field name="keyfield" type="pint" indexed="true" stored="true" docValues="true" multiValued="false"/>
+  
+  <!-- Our external file field type -->
+  <!-- Begin bad stuff: keyfield is points-based -->
+  <fieldType name="eff" class="solr.ExternalFileField" keyField="keyfield"/>
+  <!-- End bad stuff -->
+
+</schema>

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

@@ -276,7 +276,10 @@
 valued. -->
     <fieldType name="location" class="solr.LatLonType" subFieldSuffix="_coordinate"/>
 
-    <fieldType name="eff_tfloat" keyField="eff_ti" defVal="0"
+  <!-- eff_tint class can't be randomized, because ExternalFileField disallows point-based keyField.  See SOLR-10846. -->
+  <fieldType name="eff_tint" class="solr.TrieDoubleField" docValues="${solr.tests.numeric.dv}" precisionStep="8"/>
+  <field name="eff_tint" type="eff_tint" indexed="true" stored="true"/>
+  <fieldType name="eff_tfloat" keyField="eff_tint" defVal="0"
                stored="false" indexed="true"
                class="solr.ExternalFileField" />
 

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

@@ -18,7 +18,7 @@ package org.apache.solr.schema;
 
 import org.apache.commons.io.FileUtils;
 import org.apache.solr.SolrTestCaseJ4;
-import org.junit.BeforeClass;
+import org.apache.solr.common.SolrException;
 import org.junit.Test;
 
 import java.io.File;
@@ -26,12 +26,6 @@ import java.io.IOException;
 
 public class ExternalFileFieldSortTest extends SolrTestCaseJ4 {
 
-  @BeforeClass
-  public static void beforeTests() throws Exception {
-    initCore("solrconfig-basic.xml", "schema-eff.xml");
-    updateExternalFile();
-  }
-
   static void updateExternalFile() throws IOException {
     final String testHome = SolrTestCaseJ4.getFile("solr/collection1").getParent();
     String filename = "external_eff";
@@ -48,7 +42,10 @@ public class ExternalFileFieldSortTest extends SolrTestCaseJ4 {
   }
 
   @Test
-  public void testSort() {
+  public void testSort() throws Exception {
+    initCore("solrconfig-basic.xml", "schema-eff.xml");
+    updateExternalFile();
+
     addDocuments();
     assertQ("query",
         req("q", "*:*", "sort", "eff asc"),
@@ -56,4 +53,12 @@ public class ExternalFileFieldSortTest extends SolrTestCaseJ4 {
         "//result/doc[position()=2]/str[.='1']",
         "//result/doc[position()=10]/str[.='8']");
   }
+  
+  @Test
+  public void testPointKeyFieldType() throws Exception {
+    // This one should fail though, no "node" parameter specified
+    SolrException e = expectThrows(SolrException.class, 
+        () -> initCore("solrconfig-basic.xml", "bad-schema-eff.xml"));
+    assertTrue(e.getMessage().contains("has a Point field type, which is not supported."));
+  }
 }

solr/core/src/test/org/apache/solr/search/function/TestFunctionQuery.java

@@ -293,11 +293,8 @@ public class TestFunctionQuery extends SolrTestCaseJ4 {
 
   @Test
   public void testExternalFileFieldNumericKey() throws Exception {
-    assumeFalse("SOLR-10846: ExternalFileField/FileFloatSource throws NPE if keyField is Points based",
-                Boolean.getBoolean(NUMERIC_POINTS_SYSPROP));
-    
     final String extField = "eff_trie";
-    final String keyField = "eff_ti";
+    final String keyField = "eff_tint";
     assertU(adoc("id", "991", keyField, "91"));
     assertU(adoc("id", "992", keyField, "92"));
     assertU(adoc("id", "993", keyField, "93"));