Bug/ltr #260
Bug/ltr #260
Conversation
Signed-off-by: Scott Stults <[email protected]>
…es prior to starting new task\n- Files:\n * Modified: src/main/java/org/opensearch/searchrelevance/executors/ExperimentTaskManager.java\n * Added: .java-version\n * Added: src/test/scripts/esci_us_subset.ndjson Signed-off-by: Scott Stults <[email protected]>
…to avoid parse errors in Experiment path\n\n- Preprocess rescore_query in SearchRequestBuilder to base64-encode and wrap unregistered queries (e.g., LTR) using wrapper query\n- Applies to both standard and hybrid search request builders\n- Verified with JDK 21: compileJava and test tasks succeeded Signed-off-by: Scott Stults <[email protected]>
…archRequestBuilder Signed-off-by: Scott Stults <[email protected]>
Signed-off-by: Scott Stults <[email protected]>
sstults
requested review from
epugh,
fen-qin,
heemin32,
martin-gaievski and
wrigleyDan
as code owners
docs/feature-design/remote-query.md
Outdated
There was a problem hiding this comment.
I don't think we want this doc in this pR ;_)
There was a problem hiding this comment.
I don't even see a log of that in my repo! Anyway, GitHub UI let me delete it.
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| private static String extractWrappedRescoreQueryBase64(Map<String, Object> sourceMap) { |
There was a problem hiding this comment.
is all this awful nesting and mapping required? Sigh. More just of a a comment on the complexity of all of this.
There was a problem hiding this comment.
Yeah, you're right. I think we can use NamedXContentRegistry to parse any valid block as long as that plugin is registered on the node that's doing the processing. I think that's a reasonable prerequisite since we'd expect, for example, LTR to be installed on any node running an experiment with LTR re-ranked queries.
I'll see what I can do.
There was a problem hiding this comment.
Yep, that works. The unit test just checks whether rescore clauses parse (core DSL) and the IT tests LTR. We could add more IT tests for modules that add to the DSL, but just the one should be sufficient.
Signed-off-by: Scott Stults <[email protected]>
Signed-off-by: Scott Stults <[email protected]>
Signed-off-by: Scott Stults <[email protected]>
Signed-off-by: Scott Stults <[email protected]>
2 participants
Description
The bug was that our builder tried to parse an inline sltr rescore_query, which failed NamedXContent parsing and threw a [rescore_query] error. The fix base64-encodes the rescore_query payload and wraps it in a wrapper query during request build (both object and array rescore forms), letting OpenSearch parse it server‑side and eliminating the error.
Issues Resolved
Closes #255
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.