magibney commented on a change in pull request #2:
URL: https://github.com/apache/solr/pull/2#discussion_r593330235



##########
File path: solr/core/src/java/org/apache/solr/query/SolrRangeQuery.java
##########
@@ -320,16 +319,25 @@ public TermsEnum getTermsEnum(LeafReaderContext ctx) 
throws IOException {
 
   private static class SegState {
     final Weight weight;
-    final DocIdSet set;
+    final DocSet docs;
+    final DocIdSet docIdSet;

Review comment:
       I initially did that actually! but afaict there's something a little 
weird going on here where if the "DISI supplier" is built by the "builder" at 
the end of `getSegState(LeafReaderContext)`, it's actually building a 
seg-specific set that's ultimately retrieved via `setStates[context.ord]`, and 
passed a somewhat redundant `LeafReaderContext`. So I _think_ that for sets 
that get manually built in this way, they either need to be built as a single 
DocSet across all segs, or as a DocSet that pertains to a single segment only 
(if that's even a thing?). Both of those seem awkward, which is why I ended up 
sticking with the DocIdSet for this case. It's definitely a bit weird though, 
so I'm curious what you think of any alternatives.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to