Revert "Removing unused fetch sub phase processor initialization during fetch… (#12503)"

[reta]

This reverts commit da5b205.

Description

Reverts #12503 that introduced regression in 2.13.0. Thanks a lot @martijnbolhuis for spotting it and supplying the tests, @dblock for working with the contributor.

Related Issues

Closes #13467

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
• Commits are signed per the DCO using --signoff
• Commit changes are listed out in CHANGELOG.md file (See: Changelog)
• Public documentation issue/PR created

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]

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

[reta]

@jainankitk fyi, @msfroh @dblock please take a look folks (just a revert)

[jainankitk]

@reta - Thank you for this PR. Can you also include the repro as test case for identifying this going forward? I see the test case is included

Also, do we need to revert ScriptFieldsPhase change as well? I can also raise PR to just revert inner hits change if you want

[dblock]

@jainankitk Do you see a fix that avoids the revert and fixes the issue?

[jainankitk]

@jainankitk Do you see a fix that avoids the revert and fixes the issue?

We can include the changes from #13488 as part of this, or drop this and merge the other PR

[reta]

Also, do we need to revert ScriptFieldsPhase change as well? I can also raise PR to just revert inner hits change if you want

I am going to revert the change in full and ask for more tests to be added after

[jainankitk]

I am going to revert the change in full and ask for more tests to be added after

@reta - Can you check #13488 once?

[github-actions]

✅ Gradle check result for 86cfde2: SUCCESS

[codecov]

Codecov Report

Attention: Patch coverage is 50.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 71.44%. Comparing base (b15cb0c) to head (86cfde2).
Report is 247 commits behind head on main.

Files	Patch %	Lines
...earch/search/fetch/subphase/ScriptFieldsPhase.java	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #13486      +/-   ##
============================================
+ Coverage     71.42%   71.44%   +0.02%     
- Complexity    59978    60974     +996     
============================================
  Files          4985     5050      +65     
  Lines        282275   286800    +4525     
  Branches      40946    41549     +603     
============================================
+ Hits         201603   204909    +3306     
- Misses        63999    64956     +957     
- Partials      16673    16935     +262     

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

[dblock]

I am going to revert the change in full and ask for more tests to be added after

@reta - Can you check #13488 once?

It sounds like @reta and @msfroh would prefer to merge this PR vs. #13488, I'll let either of them to make that decision and hit the approve/merge buttons. I don't have a strong opinion other than I'm glad we're adding a test for the regression.