fix issue 18446 NPE by making sure scoreSupplier.get() returns non-null scorer #19650

haruki1243 · 2025-10-16T01:23:42Z

Description

fix issue 18446 NPE by making sure scoreSupplier.get() returns non-null scorer

Related Issues

Check List

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

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.

github-actions · 2025-10-16T01:45:03Z

❌ Gradle check result for 19d098a: 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?

gaobinlong · 2025-10-16T01:59:17Z

Thanks @haruki1243, the spotlessJavaCheck gradle task failed, please run ./gradlew :server:spotlessApply and then ./gradlew :server:check to make sure the main part of gradle checks can pass. Unit test and yaml rest test are needed, changelog is also needed.

server/src/main/java/org/opensearch/common/lucene/search/function/ScriptScoreQuery.java

haruki1243 · 2025-10-16T06:55:29Z

Hi @gaobinlong @owaiskazi19

Thank you so much for your reply. I have made the requested changes, including apotless formatting, tests and changelog

I added test cases to ScriptScoreQueryTests and ScriptScoreQueryIT. both pass. You can try to replace the implementation of ScriptScoreQuery with the old one, and you will see both of them fail.

Regarding yaml rest tests, sorry I didn't find tests that uses script score in those yaml tests, and ScriptScoreQueryIT seems to be the right place for its integration test instead.

github-actions · 2025-10-16T08:54:36Z

❌ Gradle check result for 29f7ccd: null

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?

haruki1243 · 2025-10-16T23:15:54Z

❌ Gradle check result for 29f7ccd: null

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?

Looks like Jenkins is down

Trigger Jenkins workflows
{"pr_from_sha": "29f7ccd3b63d053ed318383ad421472c90e06991", "pr_from_clone_url": "https://github.com/haruki1243/OpenSearch.git", "pr_to_clone_url": "https://github.com/opensearch-project/OpenSearch.git", "pr_title": "fix issue 18446 npe by making sure scoresupplierget returns nonnull scorer", "pr_number": "19650", "post_merge_action": "false", "pr_owner": "haruki1243"}

<title>403 Forbidden</title>

403 Forbidden

jq: parse error: Invalid numeric literal at line 1, column 7 QUEUE_URL wait for jenkins to start workflow Check if queue exist in Jenkins after triggering Use queue information to find build number in Jenkins if available

github-actions · 2025-10-20T10:00:19Z

❌ Gradle check result for 29798e0:

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?

…ll scorer Signed-off-by: Yao Zou <[email protected]>

Signed-off-by: Yao Zou <[email protected]> Signed-off-by: Andrew Ross <[email protected]>

github-actions · 2025-11-05T01:02:12Z

❌ Gradle check result for 9e69b55: 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?

github-actions · 2025-11-05T05:46:23Z

✅ Gradle check result for 9e69b55: SUCCESS

codecov · 2025-11-05T05:46:44Z

Codecov Report

❌ Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.20%. Comparing base (db72ca2) to head (9e69b55).
⚠️ Report is 41 commits behind head on main.

Files with missing lines	Patch %	Lines
...ommon/lucene/search/function/ScriptScoreQuery.java	78.57%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #19650      +/-   ##
============================================
+ Coverage     73.09%   73.20%   +0.11%     
- Complexity    71062    71224     +162     
============================================
  Files          5754     5766      +12     
  Lines        325260   325459     +199     
  Branches      47036    47080      +44     
============================================
+ Hits         237736   238240     +504     
+ Misses        68383    68084     -299     
+ Partials      19141    19135       -6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

vibrantvarun · 2025-11-05T16:33:09Z

@haruki1243 Thanks for fixing the issue. Could you please write a summary of the fix in the PR description like what changes specifically you made from the existing code to fix the issue.

kkhatua · 2025-11-05T16:42:48Z

@haruki1243 Is this reproducible in 2.19 as well ? We might need to port it to a future 2.19.5 version.

Tagging @jainankitk for review. Looking at the stack trace, I'm wondering if the fix should be in Lucene.

server/src/main/java/org/opensearch/common/lucene/search/function/ScriptScoreQuery.java

jainankitk

Was reviewing this change when I noticed that the PR was already merged. Couple of minor comments, but the change itself looks good to me. Also, I am still wondering about this comment #18446 (comment) regarding FunctionScoreQuery on the original issue

jainankitk · 2025-11-05T20:52:05Z

server/src/main/java/org/opensearch/common/lucene/search/function/ScriptScoreQuery.java

        }
        boolean needsScore = scriptBuilder.needs_score();
        ScoreMode subQueryScoreMode = needsScore ? ScoreMode.COMPLETE : ScoreMode.COMPLETE_NO_SCORES;
        Weight subQueryWeight = subQuery.createWeight(searcher, subQueryScoreMode, 1.0f);


Why are we not using the boost? Is it because the boost is irrelevant for the subQuery?

jainankitk · 2025-11-05T20:53:22Z

server/src/main/java/org/opensearch/common/lucene/search/function/ScriptScoreQuery.java

+                ScorerSupplier subQueryScorerSupplier = subQueryWeight.scorerSupplier(context);
+                if (subQueryScorerSupplier == null) {
+                    return null;
+                }


Ensuring the subQueryScorerSupplier is non-null here helps make the ScorerSupplier implementation cleaner and easier.

jainankitk · 2025-11-05T20:54:03Z

server/src/main/java/org/opensearch/common/lucene/search/function/ScriptScoreQuery.java

+                if (subQueryScorerSupplier == null) {
+                    return null;
+                }
+                Weight weight = this;


nit: I will probably decare subQueryScorerSupplier and weight as final.

haruki1243 · 2025-11-06T00:50:56Z

@haruki1243 Is this reproducible in 2.19 as well ? We might need to port it to a future 2.19.5 version.

Tagging @jainankitk for review. Looking at the stack trace, I'm wondering if the fix should be in Lucene.

It's not in 2.19 since it happens when opensearch upgrade lucene to version 10.

It's not a bug in lucene, since in lucene there is a contract that the scorer from scorersupplier (something added in lucene 10) cannot be null. It is opensearch that violates such contract

…ll scorer (#19650) * fix issue 18446 NPE by making sure scoreSupplier.get() returns non-null scorer Signed-off-by: Yao Zou <[email protected]> * add tests and changelog Signed-off-by: Yao Zou <[email protected]> Signed-off-by: Andrew Ross <[email protected]> --------- Signed-off-by: Yao Zou <[email protected]> Signed-off-by: Andrew Ross <[email protected]> Co-authored-by: Haruki <[email protected]> (cherry picked from commit 39b4a70) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

sandeshkr419 · 2025-11-14T22:11:32Z

@asimmahmood1 Is there a 3.3 patch release planned?
If not, there is no need to backport this to 3.3

…ll scorer (opensearch-project#19650) * fix issue 18446 NPE by making sure scoreSupplier.get() returns non-null scorer Signed-off-by: Yao Zou <[email protected]> * add tests and changelog Signed-off-by: Yao Zou <[email protected]> Signed-off-by: Andrew Ross <[email protected]> --------- Signed-off-by: Yao Zou <[email protected]> Signed-off-by: Andrew Ross <[email protected]> Co-authored-by: Haruki <[email protected]>

haruki1243 requested a review from a team as a code owner October 16, 2025 01:23

haruki1243 force-pushed the fix-issue-18446-NPE branch from e325aab to 19d098a Compare October 16, 2025 01:26

github-actions bot added bug Something isn't working lucene Search Search query, autocomplete ...etc labels Oct 16, 2025

haruki1243 changed the title ~~fix issue 18446 NPE by making sure scoreSupplier.get() returns non-nulll scorer~~ fix issue 18446 NPE by making sure scoreSupplier.get() returns non-null scorer Oct 16, 2025

owaiskazi19 reviewed Oct 16, 2025

View reviewed changes

server/src/main/java/org/opensearch/common/lucene/search/function/ScriptScoreQuery.java Show resolved Hide resolved

haruki1243 requested review from Bukhtawar, CEHENKLE, Rishikesh1159, anasalkouz, andrross, ashking94, cwperks, dbwiddis, gbbafna, jed326, kotwanikunal, mch2, msfroh, reta, sachinpkale, saratvemulapalli, shwetathareja and sohami as code owners October 16, 2025 06:48

haruki1243 force-pushed the fix-issue-18446-NPE branch from 4e4d047 to 29f7ccd Compare October 16, 2025 06:51

fix issue 18446 NPE by making sure scoreSupplier.get() returns non-nu…

541af32

…ll scorer Signed-off-by: Yao Zou <[email protected]>

andrross force-pushed the fix-issue-18446-NPE branch from 29798e0 to ff65f7e Compare November 4, 2025 23:43

add tests and changelog

9e69b55

Signed-off-by: Yao Zou <[email protected]> Signed-off-by: Andrew Ross <[email protected]>

andrross force-pushed the fix-issue-18446-NPE branch from ff65f7e to 9e69b55 Compare November 4, 2025 23:44

vibrantvarun requested a review from owaiskazi19 November 5, 2025 16:26

kkhatua requested a review from jainankitk November 5, 2025 16:42

cwperks reviewed Nov 5, 2025

View reviewed changes

server/src/main/java/org/opensearch/common/lucene/search/function/ScriptScoreQuery.java Show resolved Hide resolved

cwperks approved these changes Nov 5, 2025

View reviewed changes

kaushalmahi12 approved these changes Nov 5, 2025

View reviewed changes

cwperks merged commit 39b4a70 into opensearch-project:main Nov 5, 2025
36 of 38 checks passed

jainankitk reviewed Nov 5, 2025

View reviewed changes

asimmahmood1 mentioned this pull request Nov 5, 2025

[BUG] Script_score function null pointer in Hybrid sub queries. opensearch-project/neural-search#1624

Open

opensearch-ci-bot mentioned this pull request Nov 7, 2025

[AUTOCUT] Gradle Check Flaky Test Report for RemoteRestoreSnapshotIT #14324

Open

asimmahmood1 added the backport 3.3 Backport to 3.3 branch label Nov 14, 2025

opensearch-trigger-bot bot mentioned this pull request Nov 14, 2025

[Backport 3.3] fix issue 18446 NPE by making sure scoreSupplier.get() returns non-null scorer #20006

Open

haruki1243 deleted the fix-issue-18446-NPE branch November 17, 2025 01:21

BrewTestBot mentioned this pull request Dec 17, 2025

opensearch 3.4.0 Homebrew/homebrew-core#258986

Merged

fix issue 18446 NPE by making sure scoreSupplier.get() returns non-null scorer #19650

fix issue 18446 NPE by making sure scoreSupplier.get() returns non-null scorer #19650

Uh oh!

Conversation

haruki1243 commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

github-actions bot commented Oct 16, 2025

Uh oh!

gaobinlong commented Oct 16, 2025

Uh oh!

Uh oh!

haruki1243 commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 16, 2025

Uh oh!

haruki1243 commented Oct 16, 2025

403 Forbidden

Uh oh!

github-actions bot commented Oct 20, 2025

Uh oh!

github-actions bot commented Nov 5, 2025

Uh oh!

github-actions bot commented Nov 5, 2025

Uh oh!

codecov bot commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vibrantvarun commented Nov 5, 2025

Uh oh!

kkhatua commented Nov 5, 2025

Uh oh!

Uh oh!

Uh oh!

jainankitk left a comment

Choose a reason for hiding this comment

Uh oh!

jainankitk Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

jainankitk Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

jainankitk Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

haruki1243 commented Nov 6, 2025

Uh oh!

sandeshkr419 commented Nov 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

haruki1243 commented Oct 16, 2025 •

edited

Loading

haruki1243 commented Oct 16, 2025 •

edited

Loading

codecov bot commented Nov 5, 2025 •

edited

Loading