You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@solr.apache.org by Ishan Chattopadhyaya <ic...@gmail.com> on 2025/04/19 13:27:00 UTC
Re: [PR] SOLR-17631: Upgrade main to Lucene 10.x [solr]
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), <gi...@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-unsubscribe@solr.apache.org
>
> For queries about this service, please contact Infrastructure at:
> users@infra.apache.org
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
> For additional commands, e-mail: issues-help@solr.apache.org
>
>