dsmiley commented on code in PR #3158:
URL: https://github.com/apache/solr/pull/3158#discussion_r1948247603


##########
solr/core/src/java/org/apache/solr/search/facet/FieldUtil.java:
##########
@@ -44,27 +48,49 @@ public static SortedDocValues 
getSortedDocValues(SolrIndexSearcher searcher, Str
 
   public static SortedDocValues getSortedDocValues(
       QueryContext context, SchemaField field, QParser qparser) throws 
IOException {
-    SortedDocValues si =
-        
context.searcher().getSlowAtomicReader().getSortedDocValues(field.getName());
-    // if (!field.hasDocValues() && (field.getType() instanceof StrField || 
field.getType()
-    // instanceof TextField)) {
-    // }
-
-    return si == null ? DocValues.emptySorted() : si;
+    var reader = context.searcher().getSlowAtomicReader();
+    var dv = reader.getSortedDocValues(field.getName());
+    checkDvType(dv, field, reader);
+    return dv == null ? DocValues.emptySorted() : dv;
   }
 
   public static SortedSetDocValues getSortedSetDocValues(
       QueryContext context, SchemaField field, QParser qparser) throws 
IOException {
-    SortedSetDocValues si =
-        
context.searcher().getSlowAtomicReader().getSortedSetDocValues(field.getName());
-    return si == null ? DocValues.emptySortedSet() : si;
+    var reader = context.searcher().getSlowAtomicReader();
+    var dv = reader.getSortedSetDocValues(field.getName());
+    checkDvType(dv, field, reader);
+    return dv == null ? DocValues.emptySortedSet() : dv;
   }
 
   public static NumericDocValues getNumericDocValues(
       QueryContext context, SchemaField field, QParser qparser) throws 
IOException {
-    SolrIndexSearcher searcher = context.searcher();
-    NumericDocValues si = 
searcher.getSlowAtomicReader().getNumericDocValues(field.getName());
-    return si == null ? DocValues.emptyNumeric() : si;
+    var reader = context.searcher().getSlowAtomicReader();
+    var dv = reader.getNumericDocValues(field.getName());
+    checkDvType(dv, field, reader);
+    return dv == null ? DocValues.emptyNumeric() : dv;
+  }
+
+  private static void checkDvType(Object dv, SchemaField field, LeafReader 
reader) {

Review Comment:
   There's wasn't a class cast; there was empty/no data which ultimately leads 
to no facet results.  That's *worse* than throwing an exception (of whatever 
type; no strong opinion from me).  This is my primary addition to the PR; don't 
*just* fix the wrong algorithm choice, but ensure we throw an exception in 
cases that would previously silently produce nothing.



-- 
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...@solr.apache.org

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


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

Reply via email to