Thanks for looking into this, Jason. I'll be back working on the by the end of April.
On Sat, 19 Apr, 2025, 12:33 am gerlowskija (via GitHub), <g...@apache.org> wrote: > > gerlowskija commented on code in PR #3053: > URL: https://github.com/apache/solr/pull/3053#discussion_r2050996506 > > > ########## > > solr/core/src/test/org/apache/solr/legacy/TestMultiValuedNumericRangeQuery.java: > ########## > @@ -79,16 +76,16 @@ public void testMultiValuedNRQ() throws Exception { > "asc", format.format(lower), format.format(upper), true, > true); > LegacyNumericRangeQuery<Integer> tq = > LegacyNumericRangeQuery.newIntRange("trie", lower, upper, true, > true); > - TopScoreDocCollector trCollector = TopScoreDocCollector.create(1, > Integer.MAX_VALUE); > - TopScoreDocCollector nrCollector = TopScoreDocCollector.create(1, > Integer.MAX_VALUE); > + TopScoreDocCollector trCollector = new > TopScoreDocCollectorManager(1, 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/test/org/apache/solr/legacy/TestNumericRangeQuery32.java: > ########## > @@ -183,12 +175,12 @@ private void testRange(int precisionStep) throws > Exception { > ScoreDoc[] sd = topDocs.scoreDocs; > assertNotNull(sd); > assertEquals("Score doc count" + type, count, sd.length); > - Document doc = searcher.doc(sd[0].doc); > + Document doc = searcher.storedFields().document(sd[0].doc); > > Review Comment: > ditto, re: "IndexSearcher.doc removal/replacement" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050986335) > > > > ########## > solr/core/src/test/org/apache/solr/legacy/TestNumericRangeQuery32.java: > ########## > @@ -160,20 +153,19 @@ private void testRange(int precisionStep) throws > Exception { > String field = "field" + precisionStep; > int count = 3000; > int lower = (distance * 3 / 2) + startOffset, upper = lower + count * > distance + (distance / 3); > - LegacyNumericRangeQuery<Integer> q = > - LegacyNumericRangeQuery.newIntRange(field, precisionStep, lower, > upper, true, true); > + LegacyNumericRangeQuery<Integer> q; > for (byte i = 0; i < 2; i++) { > TopFieldCollector collector = > - TopFieldCollector.create(Sort.INDEXORDER, noDocs, > Integer.MAX_VALUE); > + new TopFieldCollectorManager(Sort.INDEXORDER, noDocs, > 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/test/org/apache/solr/handler/tagger/TaggerTestCase.java: > ########## > @@ -139,11 +140,12 @@ protected TestTag[] > pullTagsFromResponse(SolrQueryRequest req, SolrQueryResponse > NamedList<?> rspValues = rsp.getValues(); > Map<String, String> matchingNames = new HashMap<>(); > SolrIndexSearcher searcher = req.getSearcher(); > + SolrDocumentFetcher docFetcher = searcher.getDocFetcher(); > DocList docList = (DocList) rspValues.get("response"); > DocIterator iter = docList.iterator(); > while (iter.hasNext()) { > int docId = iter.next(); > - Document doc = searcher.doc(docId); > + Document doc = docFetcher.doc(docId); > > Review Comment: > ditto, re: "IndexSearcher.doc removal/replacement" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050986335) > > > > ########## > solr/core/src/test/org/apache/solr/legacy/TestNumericRangeQuery64.java: > ########## > @@ -172,20 +165,19 @@ private void testRange(int precisionStep) throws > Exception { > int count = 3000; > long lower = (distance * 3 / 2) + startOffset, > upper = lower + count * distance + (distance / 3); > - LegacyNumericRangeQuery<Long> q = > - LegacyNumericRangeQuery.newLongRange(field, precisionStep, lower, > upper, true, true); > + LegacyNumericRangeQuery<Long> q; > for (byte i = 0; i < 2; i++) { > TopFieldCollector collector = > - TopFieldCollector.create(Sort.INDEXORDER, noDocs, > Integer.MAX_VALUE); > + new TopFieldCollectorManager(Sort.INDEXORDER, noDocs, > 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/test/org/apache/solr/legacy/TestNumericRangeQuery32.java: > ########## > @@ -160,20 +153,19 @@ private void testRange(int precisionStep) throws > Exception { > String field = "field" + precisionStep; > int count = 3000; > int lower = (distance * 3 / 2) + startOffset, upper = lower + count * > distance + (distance / 3); > - LegacyNumericRangeQuery<Integer> q = > - LegacyNumericRangeQuery.newIntRange(field, precisionStep, lower, > upper, true, true); > + LegacyNumericRangeQuery<Integer> q; > for (byte i = 0; i < 2; i++) { > TopFieldCollector collector = > - TopFieldCollector.create(Sort.INDEXORDER, noDocs, > Integer.MAX_VALUE); > + new TopFieldCollectorManager(Sort.INDEXORDER, noDocs, > Integer.MAX_VALUE).newCollector(); > String type; > switch (i) { > case 0: > type = " (constant score filter rewrite)"; > - q.setRewriteMethod(MultiTermQuery.CONSTANT_SCORE_REWRITE); > + q = LegacyNumericRangeQuery.newIntRange(field, precisionStep, > lower, upper, true, true, 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/test/org/apache/solr/search/LargeFieldTest.java: > ########## > @@ -96,7 +96,7 @@ public void test() throws Exception { > assertQ(req("q", "101", "df", ID_FLD, "fl", ID_FLD)); // eager load > ID_FLD; rest are lazy > > // fetch the document; we know it will be from the documentCache, > docId 0 > - final Document d = h.getCore().withSearcher(searcher -> > searcher.doc(0)); > + final Document d = h.getCore().withSearcher(searcher -> > searcher.storedFields().document(0)); > > Review Comment: > ditto, re: "IndexSearcher.doc removal/replacement" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050986335) > > > > ########## > solr/core/src/test/org/apache/solr/search/TestCancellableCollector.java: > ########## > @@ -96,7 +88,7 @@ public void tearDown() throws Exception { > > private CancellableCollector buildCancellableCollector( > final int numHits, boolean delayStart, boolean delayCollection) { > - TopScoreDocCollector topScoreDocCollector = > TopScoreDocCollector.create(numHits, null, 1); > + TopScoreDocCollector topScoreDocCollector = new > TopScoreDocCollectorManager(numHits, null, 1).newCollector(); > > Review Comment: > ditto, re: "Collector vs. CollectorManager" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050713245) > > > > ########## > solr/core/src/test/org/apache/solr/legacy/TestNumericRangeQuery64.java: > ########## > @@ -172,20 +165,19 @@ private void testRange(int precisionStep) throws > Exception { > int count = 3000; > long lower = (distance * 3 / 2) + startOffset, > upper = lower + count * distance + (distance / 3); > - LegacyNumericRangeQuery<Long> q = > - LegacyNumericRangeQuery.newLongRange(field, precisionStep, lower, > upper, true, true); > + LegacyNumericRangeQuery<Long> q; > for (byte i = 0; i < 2; i++) { > TopFieldCollector collector = > - TopFieldCollector.create(Sort.INDEXORDER, noDocs, > Integer.MAX_VALUE); > + new TopFieldCollectorManager(Sort.INDEXORDER, noDocs, > Integer.MAX_VALUE).newCollector(); > String type; > switch (i) { > case 0: > type = " (constant score filter rewrite)"; > - q.setRewriteMethod(MultiTermQuery.CONSTANT_SCORE_REWRITE); > + q = LegacyNumericRangeQuery.newLongRange(field, precisionStep, > lower, upper, true, true, 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/test/org/apache/solr/search/TestCancellableCollector.java: > ########## > @@ -114,7 +106,7 @@ private void executeSearchTest( > TopScoreDocCollector topScoreDocCollector = > (TopScoreDocCollector) > internalCancellableCollector.getInternalCollector(); > > - assertEquals(topDocs.totalHits.value, > topScoreDocCollector.getTotalHits()); > + assertEquals(topDocs.totalHits.value(), > topScoreDocCollector.getTotalHits()); > > Review Comment: > ditto, re: "Lucene movement towards Java 'records'" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050683426) > > > > ########## > solr/core/src/test/org/apache/solr/search/TestDocSet.java: > ########## > @@ -363,6 +339,11 @@ public NumericDocValues getNormValues(String field) { > return null; > } > > + @Override > > Review Comment: > ditto, re: "sparse index skip list capability" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050785064) > > > > ########## > solr/core/src/test/org/apache/solr/schema/MyCrazyCustomField.java: > ########## > @@ -51,8 +51,7 @@ public Query getPrefixQuery(QParser parser, SchemaField > sf, String termStr) { > termStr = "foo"; > } > > - PrefixQuery query = new PrefixQuery(new Term(sf.getName(), termStr)); > - query.setRewriteMethod(sf.getType().getRewriteMethod(parser, sf)); > + PrefixQuery query = new PrefixQuery(new Term(sf.getName(), termStr), > sf.getType().getRewriteMethod(parser, sf)); > > Review Comment: > ditto, re: "RewriteMethod in ctor" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050746215) > > > > ########## > solr/core/src/test/org/apache/solr/search/TestFilteredDocIdSet.java: > ########## > @@ -125,7 +115,7 @@ public void testNullDocIdSet() throws Exception { > > // First verify the document is searchable. > IndexSearcher searcher = newSearcher(reader); > - assertEquals(1, searcher.search(new MatchAllDocsQuery(), > 10).totalHits.value); > + assertEquals(1, searcher.search(new MatchAllDocsQuery(), > 10).totalHits.value()); > > Review Comment: > ditto, re: "Lucene movement towards Java 'records'" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050683426) > > > > ########## > solr/core/src/test/org/apache/solr/legacy/TestNumericRangeQuery64.java: > ########## > @@ -195,12 +187,12 @@ private void testRange(int precisionStep) throws > Exception { > ScoreDoc[] sd = topDocs.scoreDocs; > assertNotNull(sd); > assertEquals("Score doc count" + type, count, sd.length); > - Document doc = searcher.doc(sd[0].doc); > + Document doc = searcher.storedFields().document(sd[0].doc); > > Review Comment: > ditto, re: "IndexSearcher.doc removal/replacement" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050986335) > > > > ########## > solr/core/src/test/org/apache/solr/schema/DocValuesMultiTest.java: > ########## > @@ -96,14 +96,13 @@ public void testDocValues() throws IOException { > assertEquals(0, dv.nextDoc()); > assertEquals(0, dv.nextOrd()); > assertEquals(1, dv.nextOrd()); > - assertEquals(SortedSetDocValues.NO_MORE_ORDS, dv.nextOrd()); > + assertEquals(2, dv.docValueCount()); > > Review Comment: > ditto, re: "docValue iteration" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050762921) > > > > ########## > solr/core/src/test/org/apache/solr/search/TestMinHashQParser.java: > ########## > @@ -424,7 +424,7 @@ public void testBandsWrap() throws SyntaxError { > assertEquals(4, bq.clauses().size()); > for (BooleanClause clause : bq.clauses()) { > assertEquals( > - 3, ((BooleanQuery) ((ConstantScoreQuery) > clause.getQuery()).getQuery()).clauses().size()); > + 3, ((BooleanQuery) ((ConstantScoreQuery) > clause.query()).getQuery()).clauses().size()); > > Review Comment: > ditto, re: "Lucene movement towards Java 'records'" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050683426) > > > > ########## > solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java: > ########## > @@ -3158,7 +3158,7 @@ private boolean containsClause(FuzzyQuery query, > String field, String value) { > private boolean containsClause( > BooleanQuery query, String field, String value, int boost, boolean > fuzzy) { > for (BooleanClause clause : query) { > - if (containsClause(clause.getQuery(), field, value, boost, fuzzy)) { > + if (containsClause(clause.query(), field, value, boost, fuzzy)) { > > Review Comment: > ditto, re: "Lucene movement towards Java 'records'" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050683426) > > > > ########## > solr/core/src/test/org/apache/solr/search/TestFilteredDocIdSet.java: > ########## > @@ -166,7 +156,7 @@ public Explanation explain(LeafReaderContext context, > int doc) { > } > > @Override > - public Scorer scorer(LeafReaderContext leafReaderContext) { > + public ScorerSupplier scorerSupplier(LeafReaderContext > leafReaderContext) { > > Review Comment: > ditto, re: "Scorer vs. ScorerSupplier" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050863372) > > > > ########## > solr/core/src/test/org/apache/solr/search/TestSolrCoreParser.java: > ########## > @@ -123,8 +123,8 @@ public void testHandyQuery() throws ParserException { > assertTrue(query instanceof BooleanQuery); > final BooleanQuery bq = (BooleanQuery) query; > assertEquals(2, bq.clauses().size()); > - assertTrue(bq.clauses().get(0).getQuery() instanceof > MatchAllDocsQuery); > - assertTrue(bq.clauses().get(1).getQuery() instanceof > MatchNoDocsQuery); > + assertTrue(bq.clauses().get(0).query() instanceof MatchAllDocsQuery); > > Review Comment: > ditto, re: "Lucene movement towards Java 'records'" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050683426) > > > > ########## > solr/core/src/test/org/apache/solr/search/TestMaxScoreQueryParser.java: > ########## > @@ -88,10 +88,10 @@ public void testPureMax() { > q = parse("foo bar"); > clauses = clauses(q); > assertEquals(1, clauses.length); > - assertTrue(clauses[0].getQuery() instanceof DisjunctionMaxQuery); > + assertTrue(clauses[0].query() instanceof DisjunctionMaxQuery); > > Review Comment: > ditto, re: "Lucene movement towards Java 'records'" > > See [comment here]( > https://github.com/apache/solr/pull/3053#discussion_r2050683426) > > > > -- > 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 > >
