Repository navigation
SOLR-12505: fetch() returns the fetched fields when the target handler's default query parser is not lucene - #5169
Open
nick-boss-tech wants to merge 12 commits into
Open
nick-boss-tech wants to merge 12 commits into
nick-boss-tech wants to merge 12 commits into
Conversation
The batch query uses local params that only the lucene parser honors. Send defType=lucene so a handler default such as edismax does not defeat fetch(). Adds an unrun, hypothetical regression test. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The subjects indexed by testFetchStreamWithNonLuceneDefaultDefType ended in the join keys (0, 1, 2) and are copyFielded into text, the handler default field, so the default edismax parser found the documents by accident and the test passed without the fix. The subjects are now alpha, bravo and charlie, which share no token with the keys; on base code the test fails with the fetched subject null, the ticket symptom. Also drops the stale javadoc note claiming the test was never run.
The new test built its StreamContext with a fresh SolrClientCache and never closed it, leaking the cloud client at class teardown. The cache is now closed in the finally block.
…nd keep assertThrows lambdas to one statement
…espace and name the check in the changelog title
… break the local-params value, and reject a leading '$'
nick-boss-tech
marked this pull request as ready for review
October 16, 2026 16:00
…solate the edismax test's Config API overlay in a dedicated configset
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 AI text below 🤖 (posted on behalf of Nick Shanin)
https://issues.apache.org/jira/browse/SOLR-12505
What happens today
fetch() returns the tuples without the fetched fields when the target handler's default query parser is not lucene.
fetch() sends its batch as
{! df=<join field> q.op=OR cache=false }followed by the escaped key values. It sends nodefType, so the handler's default parser reads the batch. Solr reads the{!prefix only when that default is lucene or func (QParser.java lines 387 to 396, merge-base). With another default, the batch is read as plain query text and the fetched fields are missing. The ticket's assignee, Eric Pugh, found that removingdefType=edismaxfrom the/selecthandler fixed it.The batch is built at FetchStream.java lines 239 to 246 (merge-base; current main has the same lines).
What this change does
fetch() sends
defType=lucenewith every batch, so the prefix is read whatever the handler's default is.FetchStream.java lines 273 to 276 add the parameter. Behavior change: a
defTypedefault on the handler no longer applies to fetch batches. A handler with the lucene default, which is Solr's default and the test setup, sends the same batch as before.fetch() also rejects a join field name that would break its batch query.
A join field name that would break the batch query now fails when the expression is built, and the error names the field (FetchStream.java lines 153 to 187). The batch sends the name as an unquoted value, which the local-params reader ends at whitespace or
}, so both are rejected anywhere in the name. A leading quote would switch the reader to a quoted string, and a leading$would be read as a reference to a request parameter, so both are rejected at the start of the name. The whitespace in that check isCharacter.isWhitespace, the same definition the local-params reader splits on, so Unicode spaces are rejected as well as ASCII ones. Before, a name with whitespace or}ended thedf=value early in the batch query and corrupted the fetched-field list, and a name starting with$sent the batch looking up the wrong field. The name is rejected rather than escaped: escaping would let the batch accept a name that is not in the schema, while the check fails with a message that says what is wrong.Proof
Each new test fails without its fix and passes with it.
testFetchStreamWithNonLuceneDefaultDefTypesets the test collection's/selectdefault to edismax. On the merge-base FetchStream, with the test as it stands at 5f20e11, it fails withComparisonFailure: expected:<alpha> but was:<null>. The test collection is created from a dedicated copy of the configset: the Config API overlay is stored with the configset, so setting the/selectdefault on the shared configset would outlive the test and change/selectfor the other tests in the class.With this change, StreamDecoratorTest runs 60 tests, 0 skipped, 0 failures at head
cc3b9d681e9d3caa63bb405baf5b19c66e64cd04, verified 2026-10-16. That includes the two existing fetch tests, which use the default parser and do not change.testFetchStreamRejectsJoinFieldNameThatBreaksLocalParamssends the join field namea i. On the parent commit 5f20e11, which has no join check, the drain fails with the server's parser error,invalid query parser 'i', which does not name the join field. With this change it fails at construction with the message on the cause chain (see Limits).testFetchStreamRejectsJoinFieldNameWithUnicodeSpacesends the join field namea\u2003i, with an em space (U+2003) instead of an ASCII space; the local-params reader splits on that character too. On the merge-base code, which has no join check, nothing rejects the name and no error is thrown. The ASCII-only pattern the check used before did not catch it either. With this change it fails at construction with the same message.testFetchStreamRejectsJoinFieldNameStartingWithDollarSignsends the join field name$a_i. With this change the expression fails as it is built. On the head before the leading-$rejection, nothing rejects the name: the batch is sent,$a_iis read as a parameter reference, and the server answers that parametera_iis missing.testFetchStreamRejectsJoinFieldNameWithClosingBraceOrLeadingQuotecovers the remaining rejected shapes:a}i,'a_iand"a_i. Each fails at construction with the same message.A choice to check
The batch sets
defType=lucene. The ticket thread also named the terms query parser.Joel Bernstein proposed setting the parameter, and David Smiley agreed (his comment). Smiley also suggested the terms parser (plugin name, merge-base, comma separator, merge-base). That alternative sets
defType=termswith the keys as the query. It changes the query text of every batch, and the keys would need the terms separator instead of the escaping the batch uses now.Was setting
defTypeon the batch the right call, or should the batch use the terms parser?One difference matters at scale: the batch ORs one clause per key value, so a batch holding more key values than the handler's
maxBooleanClauses(1024 by default) would fail under lucene. The terms parser has no such limit.Limits
The batch no longer depends on a handler's default defType. The test uses edismax only. Not covered: the realtime get endpoint, or a handler that pins defType under invariants.
}, and names starting with a single quote, a double quote, or$. Names containing{,!or a backslash are not rejected, because the reader treats them as ordinary characters.defTypeunderinvariantsstill overrides the batch'sdefType=lucene, because request parameters do not override invariants. The new test covers a handlerdefaultsentry; the invariants configuration is not covered.Changelog: changelog/unreleased/SOLR-12505-fetch-stream-deftype.yml
AI assistance
AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution.