Allow plugins outside core package to access and override some function of FieldMapper

[bzhangam]

Description

Allow plugin out side core package to access some functions and fields of the FieldMapper.
Allow plugin to override the merge function of the FieldMapper.

Related Issues

This is needed to support this PR for this feature.

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.

[bzhangam]

This change relax the access to this function but all its subclasses override it with protected access modifier and we also need to change them as public. I have updated all subclasses in core. But if there is any plugin extending it we also need to update the plugin accordingly. Not sure how should we communicate change like this.

[msfroh]

It's not clear to me why these need to be changed from protected to public.

Subclasses can still override protected methods, even it they're defined in a plugin.

[bzhangam]

We want to make it public to allow plugin to invoke it through a field type not override it. Since we want to delegate the query work to the delegate field type and in plugin we cannot invoke the protected function even we are in the subclass. Check this code change.

[msfroh]

Got it -- I think you would need SemanticFieldMapper to extend TermBasedFieldMapper in order to have access to the protected method. I wonder if that would make sense? It does look like you want your SemanticFieldMapper to have term-based behavior, but be decoupled from the specifics of which term-based field type is being wrapped.

[bzhangam]

You mean TermBasedFieldType? We are extending the StringFieldType which extends the TermBasedFieldType. It can access the protected method in core package. But in neural plugin we cannot access it we only can override it. But we want to delegate the query work to the wrapped field type where we don't want to override the function but invoke the function of the wrapped field type directly. To do that we need to make it publish like other functions.

[bzhangam]

This is mainly needed by KeywordFieldMapper since it overrides the default behavior of the indexedValueForSearch function. Otherwise we don't need to override it since when we are extending the StringFieldType which has a default implementation for this function.

[msfroh]

Right -- but indexedValueForSearch only needs to be implemented so that it can be called from the various *Query methods, which are declared in MappedFieldType.

If SemanticStringFieldType extends MappedFieldType, it can delegate all of the *Query methods to delegate without caring about the internal implementation details (indexedValueForSearch). Would that work?

[bzhangam]

I think we don't care about the internal implementation details we just want to delegate the work. To delegate the work we need to do something like:

    @Override
    public BytesRef indexedValueForSearch(Object value) {
        return delegateFieldType.indexedValueForSearch(value);
    }

We cannot simply rely on our semantic field type even it's already extending the string field type because it's possible the delegate field type can have new parameters. e.g. code. And it can make the semantic field type behave different from the delegate field type.

The way we can avoid this I think is to extend the delegate field type directly. e.g. If we have a semanticKeywordFieldType extending the KeywordFieldType and use the delegate keyword field type to set the same parameters for it then we don't need to override the function to delegate the query work. We can directly use it since it should behave the same as the delegate keyword field type.

[msfroh]

Right, but if SemanticStringFieldType extends MappedFieldType, then it doesn't need to delegate indexedValueForSearch, because it doesn't care that it exists (it's not part of the signature of MappedFieldType).

The fact that the delegate field type internally has an indexedValueForSearch is hidden by the delegated calls to termQuery, etc.

[bzhangam]

Synced offline with Michael. We will create a FilterFieldType in core to handle the delegate work and in plugin we simply need to extend it. This PR will not be needed.

[github-actions]

❌ Gradle check result for 1cc9d6b: 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]

❕ Gradle check result for e8c8411: UNSTABLE

• TEST FAILURES:

      1 org.opensearch.snapshots.DedicatedClusterSnapshotRestoreIT.testSnapshotWithStuckNode

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 72.36%. Comparing base (dcad6b8) to head (e8c8411).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
...org/opensearch/index/mapper/NumberFieldMapper.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #17575      +/-   ##
============================================
- Coverage     72.46%   72.36%   -0.10%     
+ Complexity    65757    65694      -63     
============================================
  Files          5311     5311              
  Lines        305001   305013      +12     
  Branches      44230    44232       +2     
============================================
- Hits         221022   220728     -294     
- Misses        65932    66217     +285     
- Partials      18047    18068      +21     

☔ 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.