Fix NPE explaining script_score with no sub-scorer

[kdelay]

Description

ScriptScoreQuery's Weight.explain() passes the sub-query scorer straight into ScriptScorer without a null check:

Scorer scorer = new ScriptScorer(this, makeScoreScript(context), subQueryWeight.scorer(context), subQueryScoreMode, 1f, explanationHolder);
int newDoc = scorer.iterator().advance(doc);

When a sub-query's weight explains a document as a match but produces no scorer for that segment, scorer.iterator() throws NullPointerException: Cannot invoke "org.apache.lucene.search.Scorer.iterator()" because "this.subQueryScorer" is null. Because explain() runs in the fetch phase, one such clause fails the whole search request for requests that succeed fine with explain/profile disabled.

scorerSupplier() in the same anonymous Weight was fixed for the equivalent case in #19650 and returns null there, meaning "no matches on this segment". This change makes explain() agree with it: a null sub-query scorer now yields Explanation.noMatch(...) wrapping the sub-query explanation instead of dereferencing the null scorer.

Testing:

• ScriptScoreQueryTests#testExplainWhenSubQueryScorerIsNull covers the scorer/explain mismatch with a sub-query whose weight always explains a match and never returns a scorer. Reverting the explain() guard makes it fail with the same NPE quoted above.
• ./gradlew :server:test --tests "*ScriptScore*" --tests "org.opensearch.index.query.functionscore.*" --tests "*FunctionScoreQuery*" passes, as do :server:spotlessJavaCheck and :server:forbiddenApisMain.

No CHANGELOG entry: the file records that it is no longer used for release notes as of 3.6 (#21071).

Related Issues

Resolves #22619

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

AI tool disclosure

Following the disclosure request in opensearch-project's CONTRIBUTING guide: I used an AI coding tool, Claude Code (Anthropic Claude), to write the change and the test in this pull request. The failure mode, the reproduction and the verification runs quoted above were checked against a local build before submitting, and I can answer questions about any part of the diff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit f802fd9)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to f802fd9
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Use scorerSupplier instead of scorer The Weight.scorer(context) method is deprecated/discouraged in Lucene in favor of scorerSupplier(context), and it internally calls scorerSupplier. Additionally, calling scorer() here may materialize the scorer eagerly. Use scorerSupplier(context) and check for null first, then call get(0) to obtain the scorer, which aligns with how scorerSupplier() elsewhere in this class handles the null case. server/src/main/java/org/opensearch/common/lucene/search/function/ScriptScoreQuery.java [185-191] -Scorer subQueryScorer = subQueryWeight.scorer(context); -if (subQueryScorer == null) { +ScorerSupplier subQueryScorerSupplier = subQueryWeight.scorerSupplier(context); +if (subQueryScorerSupplier == null) { // The sub-query explains this document as a match but produces no scorer for this segment. - // scorerSupplier() treats that as "no matches on this segment", so explain() reports no match - // as well instead of dereferencing a null scorer. return Explanation.noMatch("sub-query produced no scorer for this segment", subQueryExplanation); } +Scorer subQueryScorer = subQueryScorerSupplier.get(0L); Suggestion importance[1-10]: 6 __ Why: Using scorerSupplier is more aligned with modern Lucene practice and matches the pattern used elsewhere in the class (as evidenced by the test's MatchingExplanationNullScorerQuery returning null from scorerSupplier). This is a reasonable code quality improvement, though functionally the current code works.	Low

---

Previous suggestions

Suggestions up to commit 771c576

Category	Suggestion	Impact
Possible issue	Use scorerSupplier to detect missing scorer The Weight.scorer(context) method is deprecated/discouraged in newer Lucene versions in favor of scorerSupplier(context), and calling scorer() directly may itself return null via a different code path than scorerSupplier. To be consistent with the rest of the class and correctly handle the "no scorer supplier" case described in the comment, obtain the scorer via scorerSupplier(context) and null-check the supplier before calling get(). server/src/main/java/org/opensearch/common/lucene/search/function/ScriptScoreQuery.java [185-191] -Scorer subQueryScorer = subQueryWeight.scorer(context); +ScorerSupplier subQueryScorerSupplier = subQueryWeight.scorerSupplier(context); +if (subQueryScorerSupplier == null) { + return Explanation.noMatch("sub-query produced no scorer for this segment", subQueryExplanation); +} +Scorer subQueryScorer = subQueryScorerSupplier.get(Long.MAX_VALUE); if (subQueryScorer == null) { - // The sub-query explains this document as a match but produces no scorer for this segment. - // scorerSupplier() treats that as "no matches on this segment", so explain() reports no match - // as well instead of dereferencing a null scorer. return Explanation.noMatch("sub-query produced no scorer for this segment", subQueryExplanation); } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that scorerSupplier() is the preferred API and is what scorerSupplier() in ScriptScoreQuery uses elsewhere; aligning explain() with the same path makes the null-check semantics consistent with the test case (which returns null from scorerSupplier). This is a valid consistency and correctness improvement.	Medium

[github-actions]

❌ Gradle check result for 771c576: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[kdelay]

The failure in build 82896 is in RestoreShallowSnapshotV2IT, which this change does not touch:

org.opensearch.remotestore.RestoreShallowSnapshotV2IT.testRestoreInSameRemoteStoreEnabledIndex {p0={"opensearch.experimental.feature.writable_warm_index.enabled":"true"}}
org.opensearch.remotestore.RestoreShallowSnapshotV2IT.classMethod

That build reported 2 failed, 43005 passed, 752 skipped, and both failures are in that one suite.

This suite is tracked as flaky in #16658. Querying the Gradle Check Metrics data for RestoreShallowSnapshotV2IT since 2026-07-03, it failed in 17 distinct gradle-check builds, 7 of which had no pull request attached: post-merge builds 82756 (2026-07-29) and 81986 (2026-07-10), and timer builds 82893 and 82890 (2026-08-01), 82279 (2026-07-18), 82031 (2026-07-11), 81967 (2026-07-09). So it fails on main without any change from a PR.

On the mechanism side, the diff here adds one branch to ScriptScoreQuery.explain() that is taken only when the sub-query returns no scorer for the segment; on every other path the same scorer instance is passed to ScriptScorer as before. The failing suite exercises remote-store snapshot restore and does not go through script scoring.

I have pushed a commit to re-run the check.

[github-actions]

Persistent review updated to latest commit f802fd9

[github-actions]

❌ Gradle check result for f802fd9: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[kdelay]

Build 82898 failed on a different suite than 82896: Netty4HttpRequestSizeLimitIT, 1 failed / 31138 passed / 267 skipped.

org.opensearch.http.netty4.Netty4HttpRequestSizeLimitIT.testLimitsInFlightRequests

This one is already tracked as flaky in #18875 (open). Querying the Gradle Check Metrics data for Netty4HttpRequestSizeLimitIT since 2026-07-03, it failed in 109 distinct gradle-check builds, 49 of which had no pull request attached (42 timer builds, 7 post-merge builds). The most recent non-PR occurrences are builds 82888, 82887, 82884 and 82882, all within the last two days, so it is failing on main independently of this branch.

On the mechanism side, the only source change here adds one branch to ScriptScoreQuery.explain() that is taken when the sub-query returns no scorer for a segment. The failing test exercises HTTP request size limiting in the netty4 transport and never reaches script scoring.

That makes two consecutive runs failing in two separately tracked flaky suites (RestoreShallowSnapshotV2IT in 82896, this one in 82898), so I have not pushed another no-op commit to spin the check again. I am happy to re-push or rebase whenever a fresh run is useful.

[hyunwoo-kurly]

Thanks for picking this up.

For context on the wider defect: the same unguarded pattern exists in FunctionScoreQuery.CustomBoostFactorWeight.explain(), where functionScorer(context) returns null for the same state and the result is dereferenced without a check. I filed #22634 and opened #22635 with a fix and a regression test for that site. It is a different file, so there is no overlap with this PR.

Two notes that may be useful while reviewing this one:

• The other half of the mismatch is a sub-query weight that explains a match while returning a scorer which is not null but does not position on the document. advance(doc) then returns a different doc and the code continues into assert doc == newDoc and scorer.score(), so with assertions enabled it trips the assert, and without them the explanation reports the score of a different document. I left the equivalent assert alone in #22635 for the same reason, and I am handling that state as a separate change on top of both PRs.
• A concrete producer of that state is the k-NN plugin's KNNWeight.explain(). On 2.19 it is return Explanation.match(1.0f, "No Explanation");, a match for every document without consulting a scorer at all. On main the radial-search path returns Explanation.match(0.0f, ...) for a document that is not among the segment's nearest neighbors. That is the k-NN side of the disagreement, tracked in opensearch-project/k-NN#3479 with a fix in opensearch-project/k-NN#3480.

[kdelay]

Thanks for the follow-up, and for filing #22634 / #22635. Agreed there is no overlap: this PR touches only ScriptScoreQuery.java and its test, and #22635 is confined to FunctionScoreQuery.java.

On the other half of the mismatch, a sub-query weight that explains a match while returning a scorer that does not position on the document: I probed where that lands on this branch today. With a sub-query whose scorerSupplier hands back a scorer over an empty iterator, explain() walks straight past the null guard added here and fails at assert doc == newDoc with an AssertionError. So it is a genuinely separate failure mode, and this change neither fixes nor masks it.

Past that point I am reading the code rather than reporting a measurement: ScriptScorer.docID() delegates to the sub-query scorer, so with assertions disabled scoreScript.setDocument() would receive the advanced doc id instead of doc, which is the wrong-document score you described. I did not measure that path because the test harness here refuses to run with assertions off.

That lines up with handling it as a change on top of both PRs, so I will leave the assert alone here and keep this one to the null case. The branch is still even with main at 20ba4aa; gradle-check remains red only on the two separately tracked flaky suites noted above.