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