gerlowskija commented on code in PR #3053:
URL: https://github.com/apache/solr/pull/3053#discussion_r2050834136


##########
solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java:
##########
@@ -1240,9 +1240,9 @@ && allSameQueryStructure(lst)) {
                   BooleanQuery booleanQuery = (BooleanQuery) 
boostQuery.getQuery();
                   subs.add(
                       new BoostQuery(
-                          booleanQuery.clauses().get(c).getQuery(), 
boostQuery.getBoost()));
+                          booleanQuery.clauses().get(c).query(), 
boostQuery.getBoost()));

Review Comment:
   ditto, re: "Lucene movement towards Java 'records'"
   
   See [comment 
here](https://github.com/apache/solr/pull/3053#discussion_r2050683426)



##########
solr/core/src/java/org/apache/solr/search/GraphTermsQParserPlugin.java:
##########
@@ -200,7 +189,7 @@ public int getCost() {
     public void setCost(int cost) {}
 
     @Override
-    public Query rewrite(IndexReader reader) throws IOException {
+    public Query rewrite(IndexSearcher searcher) throws IOException {

Review Comment:
   ditto, re: "rewrite API change"
   
   See [comment 
here](https://github.com/apache/solr/pull/3053#discussion_r2050789458)



##########
solr/core/src/java/org/apache/solr/search/Grouping.java:
##########
@@ -905,11 +868,11 @@ protected Collector createFirstPassCollector() throws 
IOException {
       Collector subCollector;
       if (withinGroupSort == null || withinGroupSort.equals(Sort.RELEVANCE)) {
         subCollector =
-            topCollector = TopScoreDocCollector.create(groupDocsToCollect, 
Integer.MAX_VALUE);
+            topCollector = new TopScoreDocCollectorManager(groupDocsToCollect, 
Integer.MAX_VALUE).newCollector();

Review Comment:
   ditto, re: "Collector vs. CollectorManager"
   
   See [comment 
here](https://github.com/apache/solr/pull/3053#discussion_r2050713245) 



##########
solr/core/src/java/org/apache/solr/search/JoinQuery.java:
##########
@@ -330,7 +323,7 @@ public DocSet getDocSetEnumerate() throws IOException {
             for (int subindex = 0; subindex < numSubs; subindex++) {
               MultiPostingsEnum.EnumWithSlice sub = subs[subindex];
               if (sub.postingsEnum == null) continue;
-              int base = sub.slice.start;
+              int base = sub.slice.start();

Review Comment:
   ditto, re: "Lucene movement towards Java 'records'"
   
   See [comment 
here](https://github.com/apache/solr/pull/3053#discussion_r2050683426)



##########
solr/core/src/java/org/apache/solr/query/SolrRangeQuery.java:
##########
@@ -521,39 +511,59 @@ private SegState getSegState(LeafReaderContext context) 
throws IOException {
       return segStates[context.ord] = new SegState(segSet);
     }
 
-    private Scorer scorer(DocIdSet set) throws IOException {
+    private Scorer scorerInternal(DocIdSet set) throws IOException {
       if (set == null) {
         return null;
       }
       final DocIdSetIterator disi = set.iterator();
       if (disi == null) {
         return null;
       }
-      return new ConstantScoreScorer(this, score(), scoreMode, disi);
+      return new ConstantScoreScorer(score(), scoreMode, disi);
     }
 
-    @Override
-    public BulkScorer bulkScorer(LeafReaderContext context) throws IOException 
{
+    private static final Logger log = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+    public Scorer scorerInternal(LeafReaderContext context) throws IOException 
{
       final SegState weightOrBitSet = getSegState(context);
+      log.info("Query: " + getQuery());
+      log.info("weight: " + weightOrBitSet.weight);
+      if (weightOrBitSet.weight!=null) log.info("weight's scorer: " + 
weightOrBitSet.weight.scorer(context));
+      log.info("set: " + weightOrBitSet.set);
       if (weightOrBitSet.weight != null) {
-        return weightOrBitSet.weight.bulkScorer(context);
+        Scorer ret = weightOrBitSet.weight.scorer(context);
+        return ret;
       } else {
-        final Scorer scorer = scorer(weightOrBitSet.set);
-        if (scorer == null) {
-          return null;
-        }
-        return new DefaultBulkScorer(scorer);
+        return scorerInternal(weightOrBitSet.set);
       }
     }
 
     @Override
-    public Scorer scorer(LeafReaderContext context) throws IOException {
-      final SegState weightOrBitSet = getSegState(context);
-      if (weightOrBitSet.weight != null) {
-        return weightOrBitSet.weight.scorer(context);
-      } else {
-        return scorer(weightOrBitSet.set);
+    public ScorerSupplier scorerSupplier(LeafReaderContext context) throws 
IOException {

Review Comment:
   [INFO] The "ScoreSupplier" interface and method was added to Lucene a while 
back as a way to do better query-planning for range queries, but 
https://github.com/apache/lucene/issues/13180 recently shifted the method to 
being "abstract" to force all Weight subclasses to implement it.
   
   `scorer(..)` now has a default implementation that delegates to 
scoreSupplier, so in effect most Weight impl's have had to switch out their 
`scorer(...)` method for a `scorerSupplier(...)`.



##########
solr/core/src/java/org/apache/solr/search/Grouping.java:
##########
@@ -437,32 +425,7 @@ public void execute() throws IOException {
    */
   private void searchWithTimeLimiter(final Query filterQuery, Collector 
collector)
       throws IOException {
-    if (cmd.getTimeAllowed() > 0) {
-      if (timeLimitingCollector == null) {
-        timeLimitingCollector =
-            new TimeLimitingCollector(
-                collector, TimeLimitingCollector.getGlobalCounter(), 
cmd.getTimeAllowed());
-      } else {
-        /*
-         * This is so the same timer can be used for grouping's multiple 
phases.
-         * We don't want to create a new TimeLimitingCollector for each phase 
because that would
-         * reset the timer for each phase.  If time runs out during the first 
phase, the
-         * second phase should timeout quickly.
-         */
-        timeLimitingCollector.setCollector(collector);
-      }
-      collector = timeLimitingCollector;
-    }
-    try {
-      searcher.search(QueryUtils.combineQueryAndFilter(query, filterQuery), 
collector);
-    } catch (TimeLimitingCollector.TimeExceededException
-        | ExitableDirectoryReader.ExitingReaderException x) {
-      // INFO log the (possibly quite long) query object separately
-      log.info("Query: {}; ", query);
-      // to make WARN logged exception content more visible
-      log.warn("Query: {}; ", query.getClass().getName(), x);
-      qr.setPartialResults(true);
-    }
+    searcher.search(filterQuery, collector);

Review Comment:
   [Q] Is this logic moving somewhere else?  I'm not sure how/why we're nuking 
all of this logic?



##########
solr/core/src/java/org/apache/solr/search/EarlyTerminatingSortingCollector.java:
##########
@@ -96,7 +96,7 @@ public EarlyTerminatingSortingCollector(Collector in, Sort 
sort, int numDocsToCo
 
   @Override
   public LeafCollector getLeafCollector(LeafReaderContext context) throws 
IOException {
-    Sort segmentSort = context.reader().getMetaData().getSort();
+    Sort segmentSort = context.reader().getMetaData().sort();

Review Comment:
   ditto, re: "Lucene movement towards Java 'records'"
   
   See [comment 
here](https://github.com/apache/solr/pull/3053#discussion_r2050683426)



##########
solr/core/src/java/org/apache/solr/search/Grouping.java:
##########
@@ -623,8 +586,8 @@ protected NamedList<Object> commonResponse() {
     }
 
     protected DocList getDocList(GroupDocs<?> groups) {
-      assert groups.totalHits.relation == TotalHits.Relation.EQUAL_TO;
-      int max = Math.toIntExact(groups.totalHits.value);
+      assert groups.totalHits().relation() == TotalHits.Relation.EQUAL_TO;

Review Comment:
   ditto, re: "Lucene movement towards Java 'records'"
   
   See [comment 
here](https://github.com/apache/solr/pull/3053#discussion_r2050683426)



##########
solr/core/src/java/org/apache/solr/search/MatchCostQuery.java:
##########
@@ -66,8 +57,8 @@ public int hashCode() {
   }
 
   @Override
-  public Query rewrite(IndexReader reader) throws IOException {
-    final Query rewrite = delegate.rewrite(reader);
+  public Query rewrite(IndexSearcher searcher) throws IOException {

Review Comment:
   ditto, re: "rewrite API change"
   
   See [comment 
here](https://github.com/apache/solr/pull/3053#discussion_r2050789458)



##########
solr/core/src/java/org/apache/solr/search/JoinQuery.java:
##########
@@ -80,9 +73,9 @@ public Query getQuery() {
   }
 
   @Override
-  public Query rewrite(IndexReader reader) throws IOException {
+  public Query rewrite(IndexSearcher searcher) throws IOException {

Review Comment:
   ditto, re: "rewrite API change"
   
   See [comment 
here](https://github.com/apache/solr/pull/3053#discussion_r2050789458)



##########
solr/core/src/java/org/apache/solr/search/ExportQParserPlugin.java:
##########
@@ -94,10 +94,10 @@ public Weight createWeight(IndexSearcher searcher, 
ScoreMode scoreMode, float bo
     }
 
     @Override
-    public Query rewrite(IndexReader reader) throws IOException {
-      Query q = mainQuery.rewrite(reader);
+    public Query rewrite(IndexSearcher searcher) throws IOException {

Review Comment:
   ditto, re: "rewrite API change"
   
   See [comment 
here](https://github.com/apache/solr/pull/3053#discussion_r2050789458)



##########
solr/core/src/java/org/apache/solr/search/LegacyNumericRangeQueryBuilder.java:
##########
@@ -112,7 +113,7 @@ public Query getQuery(Element e) throws ParserException {
                 (lowerTerm == null ? null : Integer.valueOf(lowerTerm)),
                 (upperTerm == null ? null : Integer.valueOf(upperTerm)),
                 lowerInclusive,
-                upperInclusive);
+                upperInclusive, MultiTermQuery.CONSTANT_SCORE_REWRITE);

Review Comment:
   ditto, re: "RewriteMethod in ctor"
   
   See [comment 
here](https://github.com/apache/solr/pull/3053#discussion_r2050746215)



##########
solr/core/src/java/org/apache/solr/search/MatchCostQuery.java:
##########
@@ -99,16 +90,16 @@ public Explanation explain(LeafReaderContext context, int 
doc) throws IOExceptio
       // scorer() so that we can wrap TPI
 
       @Override
-      public Scorer scorer(LeafReaderContext context) throws IOException {
+      public ScorerSupplier scorerSupplier(LeafReaderContext context) throws 
IOException {
         final Scorer scorer = weight.scorer(context);
         if (scorer == null) {
           return null;
         }
         final TwoPhaseIterator tpi = scorer.twoPhaseIterator();
         if (tpi == null || tpi.matchCost() == matchCost) {
-          return scorer; // needn't wrap/delegate
+          return new SolrDefaultScorerSupplier(scorer); // needn't 
wrap/delegate
         }
-        return new Scorer(weight) { // pass delegated weight
+        return new SolrDefaultScorerSupplier(new Scorer() { // pass delegated 
weight

Review Comment:
   [Q] This doesn't actually use the delegated weight, as the comment 
suggests...is that intentional?



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