Skip to content

Conversation

@sjs004
Copy link
Contributor

@sjs004 sjs004 commented Sep 27, 2025

Description

CustomUnifiedHighlighter.rewriteCustomQuery() wasn't handling nested query (internally written as OpenSearchToParentBlockJoinQuery). Therefore unified highlighter wasn't working. Added the missing handling & updated tests

Related Issues

Resolves #19106

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.

@sjs004 sjs004 requested a review from a team as a code owner September 27, 2025 23:00
@sjs004 sjs004 changed the title Draft: 19106 - Fix for Unified highlighter with nested fields when match_phrase_prefix Draft: Fix for Unified highlighter with nested fields when match_phrase_prefix Sep 27, 2025
@github-actions
Copy link
Contributor

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

@sjs004 sjs004 force-pushed the fix-unified-highlighter-nested-fields branch from d61fbdb to bc6deeb Compare September 28, 2025 13:48
@github-actions
Copy link
Contributor

❌ Gradle check result for 17794cc: 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?

@sjs004 sjs004 changed the title Draft: Fix for Unified highlighter with nested fields when match_phrase_prefix Fix Unified highlighter for nested fields when using matchPhrasePrefixQuery Sep 30, 2025
@sjs004 sjs004 force-pushed the fix-unified-highlighter-nested-fields branch 2 times, most recently from 708767d to 739a4b9 Compare September 30, 2025 20:19
@sjs004 sjs004 force-pushed the fix-unified-highlighter-nested-fields branch 3 times, most recently from 2d8c6dd to bf35e4b Compare October 29, 2025 20:07
@sjs004 sjs004 requested a review from sandeshkr419 October 29, 2025 20:08
@github-actions
Copy link
Contributor

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

@sandeshkr419
Copy link
Member

sandeshkr419 commented Oct 30, 2025

Some indentation failures @sjs004.

Run './gradlew :server:spotlessApply' to fix these violations.

@sjs004 sjs004 force-pushed the fix-unified-highlighter-nested-fields branch from bf35e4b to a0ad332 Compare October 30, 2025 17:06
@github-actions
Copy link
Contributor

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

@sjs004
Copy link
Contributor Author

sjs004 commented Oct 30, 2025

@sandeshkr419 There is a failing test (org.opensearch.action.admin.cluster.filecache.PruneFileCacheIT.testPruneCacheWithRealData). Let me check it. Doesn't seem related to my change though. Maybe a flaky test

Update: It passed locally so it's a flaky test

#19724

@github-actions
Copy link
Contributor

❌ Gradle check result for 5fbbc66: 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
Copy link
Contributor

❌ Gradle check result for 13a511d: 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
Copy link
Contributor

❌ Gradle check result for 13a511d: 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?

…xQuery

Signed-off-by: sjs004 <simarjeet.singh004@gmail.com>
Signed-off-by: sjs004 <simarjeet.singh004@gmail.com>
Signed-off-by: sjs004 <simarjeet.singh004@gmail.com>
Signed-off-by: sjs004 <simarjeet.singh004@gmail.com>
Signed-off-by: sjs004 <simarjeet.singh004@gmail.com>
@sjs004 sjs004 force-pushed the fix-unified-highlighter-nested-fields branch from 13a511d to 3fc81c4 Compare October 31, 2025 10:32
@github-actions
Copy link
Contributor

✅ Gradle check result for 3fc81c4: SUCCESS

Copy link
Member

@sandeshkr419 sandeshkr419 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for working on this and adding up test cases for full coverage!

@sjs004
Copy link
Contributor Author

sjs004 commented Oct 31, 2025

Thanks @sandeshkr419 . Finally all checks have passed. I actually had to retry pipeline multiple times due to flaky tests & sometimes jenkins job for gradle-check not starting

@sandeshkr419 sandeshkr419 merged commit 0a77be1 into opensearch-project:main Oct 31, 2025
33 checks passed
@sandeshkr419
Copy link
Member

Yeah, I guess the CI was throttled up and required manual approvals to continue.

@sjs004
Copy link
Contributor Author

sjs004 commented Oct 31, 2025

@sandeshkr419 I think wait time for Jenkins job parse error can be reduced from 2 hours. It is currently 2 hours, same as wait time for status check. Usually people may assume that tests are running but in reality the job itself didn't trigger

@sandeshkr419
Copy link
Member

I think wait time for Jenkins job parse error can be reduced from 2 hours. It is currently 2 hours, same as wait time for status check. Usually people may assume that tests are running but in reality the job itself didn't trigger

@gaiksaya, @rishabh6788 - Do you think there is an opportunity to improve this?

anandpatel9998 pushed a commit to anandpatel9998/OpenSearch that referenced this pull request Nov 3, 2025
@sandeshkr419 sandeshkr419 added the v3.4.0 Issues and PRs related to version 3.4.0 label Nov 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

good first issue Good for newcomers Search:Resiliency v3.4.0 Issues and PRs related to version 3.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Unified highlighter does not highlight nested fields when match_phrase_prefix is used

3 participants