mkhludnev commented on code in PR #3158:
URL: https://github.com/apache/solr/pull/3158#discussion_r1948229222
##########
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:
It's not clear why to have this 20 lines. If we remove it, I think it just
fails at class cast somewhere later. Why ISE is better than ClassCastException
later? It's just not obvious.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
