[Backport 2.x] Introduce Template query

[opensearch-trigger-bot]

Backport 13ab4ec from #16818.

[github-actions]

✅ Gradle check result for 0688385: SUCCESS

[codecov]

Codecov Report

Attention: Patch coverage is 82.64463% with 21 lines in your changes missing coverage. Please review.

Project coverage is 71.97%. Comparing base (421f211) to head (0688385).

Files with missing lines	Patch %	Lines
...pensearch/index/query/QueryCoordinatorContext.java	58.82%	7 Missing ⚠️
...g/opensearch/index/query/TemplateQueryBuilder.java	90.16%	3 Missing and 3 partials ⚠️
...pensearch/index/query/BaseQueryRewriteContext.java	88.57%	2 Missing and 2 partials ⚠️
...java/org/opensearch/index/query/QueryBuilders.java	0.00%	1 Missing ⚠️
...rg/opensearch/index/query/QueryRewriteContext.java	0.00%	1 Missing ⚠️
...rch/search/pipeline/PipelineProcessingContext.java	0.00%	1 Missing ⚠️
...g/opensearch/search/pipeline/PipelinedRequest.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##                2.x   #17105      +/-   ##
============================================
+ Coverage     71.90%   71.97%   +0.07%     
- Complexity    65701    65737      +36     
============================================
  Files          5322     5325       +3     
  Lines        306191   306276      +85     
  Branches      44650    44656       +6     
============================================
+ Hits         220177   220453     +276     
+ Misses        67668    67386     -282     
- Partials      18346    18437      +91     

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

[msfroh]

I believe with the failure from the "Detect Breaking Changes" action, we can't backport this change to 2.x.

As @dbwiddis called out on the original issue, we're making changes to classes that are marked as @PublicAPI (even though they probably shouldn't be).

My suggestion would be to close this PR and open a new one against main to move the changelog entry from 2.x to 3.0.

[mingshl]

@reta I am looking at this PR 09bacee I wonder why this PR changed a lot of the internal classes into public API? I still do not agree that changing this QueryShardContext class is considering as a breaking change

[reta]

@reta I am looking at this PR 09bacee I wonder why this PR changed a lot of the internal classes into public API? I still do not agree that changing this QueryShardContext class is considering as a breaking change

@mingshl please check this issue for the motivation (#8127), in the nutshell this is how it works, by example of Plugin class:

• every plugin has to inherit from Plugin class, this is public API
• every method on the Plugin exposes some data types (classes, interfaces, enumerations, ...), this is public API
• every data type from above exposes some data types (classes, interfaces, enumerations, ...), this is public API (the chain continues)

Does it make sense to you? Thank you.

[mingshl]

thanks @reta! I see. This change might impact neural search plugin and learn to rank

[reta]

thanks @reta! I see. This change might impact neural search plugin and learn to rank

Yeah, our promise is to not have breaking API changes in the minor releases, these the plugins that you know will be impacted, but there are potentially many plugins that we don't know about but should not break.

[msfroh]

Yeah, our promise is to not have breaking API changes in the minor releases, these the plugins that you know will be impacted, but there are potentially many plugins that we don't know about but should not break.

In theory the plugins can and should take a QueryRewriteContext as a parameter for their doRewrite methods (if they implement new QueryBuilders), but they probably shouldn't be constructing them. (Since none of the existing method signatures on QueryRewriteContext changed, they should be fine.) Unfortunately, we don't know if plugins are constructing QueryRewriteContexts, so we have to assume that they are.

We really need to adopt JPMS one day to clarify what classes are "public API". Until then, we can mostly only assume that any public class is being used by some plugin somewhere.

[andrross]

In theory the plugins can and should take a QueryRewriteContext as a parameter for their doRewrite methods (if they implement new QueryBuilders), but they probably shouldn't be constructing them.

@msfroh I believe we relaxed the compatibility check to exclude constructors for exactly this reason. However the change from a concrete class to an interface is probably no good. We might be able to massage this in to a compatible change if that's desired. Or we can just punt it to the 3.0 release.

[dbwiddis]

We might be able to massage this in to a compatible change if that's desired.

When writing the SdkClient for multi-tenancy, we experienced a similar issue, wanting an interface that subclasses could implement, but we needed a concrete class for injection in create components.

The reason for needing the concrete class is different but the solution is the same.

1. Create the desired interface like QueryShardContextDelegate (rename the existing interface from the PR)
2. Take the current implementation that implements QueryShardContext and change the inheritance to QueryShardContextDelegate
3. Create a "new" QueryShardContext class (API). Have a setter/constructor/however you get it in there way of putting the implementation class in as a class field.
4. Copy all the method signatures from the interface, and implement them with a call to the identical method on the delegate.

See example of this pattern:

• Concrete class https://github.com/opensearch-project/opensearch-remote-metadata-sdk/blob/main/core/src/main/java/org/opensearch/remote/metadata/client/SdkClient.java
• Interface that subclasses will implement: https://github.com/opensearch-project/opensearch-remote-metadata-sdk/blob/main/core/src/main/java/org/opensearch/remote/metadata/client/SdkClientDelegate.java

CC: @mingshl