SOLR-18371: remove the deprecated pre-QueryCommand SolrIndexSearcher methods - #4754
Conversation
…methods Nine live deprecated members go: search(QueryResult, QueryCommand), three getDocList overloads and five getDocListAndSet overloads, plus three doc(...) overloads that were already commented out with stale javadoc. @deprecated count 12 -> 0, verified by listing each exact signature against origin/main, not by counting. Only five call sites in the whole tree: four in MoreLikeThisHandler, one in HighlighterTest. A naive grep for getDocList/getDocListAndSet returns ~100 hits, almost all of them the surviving overloads or unrelated classes. Two small cleanups the removal made possible: search(QueryCommand)'s body is now the two lines the deprecated overload contained, and getDocListC builds its own QueryResult instead of taking one as an always-fresh, discarded-by-callers out-parameter - resolving its own "TODO don't take QueryResult as arg" comment. Verified: compileJava/compileTestJava, ecjLintMain/Test, spotlessJavaCheck, renderJavadoc, and MoreLikeThisHandlerTest, MoreLikeThisComponentTest, HighlighterTest, TestSolrQueryParser, TestDistributedSearch - 84 tests, 0 failures. AI-assisted (Claude Sonnet 5)
| results.docList = searcher.getDocList(boostedMLTQuery, filters, null, start, rows, flags); | ||
| } | ||
| return results; | ||
| // setNeedDocSet must follow setFlags: it sets or clears GET_DOCSET within the flags |
There was a problem hiding this comment.
Ugh... wow, that sounds like a "foot gun"... is there anyway to prevent someone from making this mistake? Or, is it so localized to just this class that no one else will accidetnally do setNeedDocSet before setFlags?
Any ideas?
There was a problem hiding this comment.
setFlags ORs bits in; setNeedDocSet OR's/AND-NOTs the GET_DOCSET bit in that same field. Order only matters for the false case — clearing, then a later setFlags whose value includes that bit, would undo the clear. Here flags only ever carries GET_SCORES, so it can't actually collide.
| .setFlags(flags); | ||
| QueryResult qr = new QueryResult(); | ||
| newSearcher.getDocListC(qr, qc); | ||
| // called for its cache side effect; the returned QueryResult is unused |
| .setQuery(query) | ||
| .setOffset(0) | ||
| .setLen(1) | ||
| .setNeedDocSet(true) |
There was a problem hiding this comment.
should there have been setFlags first??
There was a problem hiding this comment.
No — there's no setFlags(...) call in this chain at all, so there's nothing for setNeedDocSet(true) to conflict with. It just OR's GET_DOCSET onto the default (0) flags.
epugh
left a comment
epugh
left a comment
There was a problem hiding this comment.
SOme questions.. love how much code this one removes!
|
cc @dsmiley -- this removes AI-assisted (Claude Sonnet 5) |
2 participants
https://issues.apache.org/jira/browse/SOLR-18371
Removes all 9 deprecated
SolrIndexSearchermembers the ticket names —search(QueryResult, QueryCommand)plus 8getDocList/getDocListAndSetoverloads — and the 3 already-commented-outdoc(...)blocks with their stale javadoc.@Deprecatedcount 12 → 0, verified by listing each exact signature againstorigin/main, not by counting. One correction to the ticket text: none of the 9 carry an explicit@deprecated Use X insteadjavadoc sentence — the replacement is visible only in each method's own body, not its documentation.5 in-tree call sites migrated (4 in
MoreLikeThisHandler, 1 inHighlighterTest) — the only ones in the whole tree; a naive grep returns ~100 hits, almost all surviving overloads or unrelated classes. 84 tests, 0 failures.On the ticket's own caution about third-party usage: no
@lucene.experimental/internal markers, no ref-guide/upgrade-notes mentions either way. The deprecation shipped in 10.0.0 (#2524, 2024-07-03), so it's had a real release's exposure.Two small cleanups made possible by the removal:
search(QueryCommand)'s body is now the two lines the deprecated overload contained, andgetDocListCbuilds its ownQueryResultinstead of taking one as an always-fresh, discarded-by-callers out-parameter — resolving the// TODO don't take QueryResult as argcomment that sat on it.AI-assisted (Claude Sonnet 5)