Revert changes in AbstractPointGeometryFieldMapper

[heemin32]

Description

The change made in AbstractPointGeometryFieldMapper class with commit heemin32@0503897 introduced a performace degradation during point data indexing. Reverting it therefore.

The change was about consolidating array format parsing code for point type in a single place to acheive following benefits.

1. Allow plugins to override array parsing logic. Plugins can add its own parsing logic for point field by providing object parser. However, it cannot override array format. Therefore, plugin have to use whatever implemented in AbstractPointGeometryFieldMapper class.
2. Enhanced code quality by removing duplicated code

There is no change in functionality because 1. There is no change in functionality in OpenSearch and 2. No plugins have its own parsing logic for point data in array format yet.

Issues Resolved

opensearch-project/opensearch-build#2888

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Commits are signed per the DCO using --signoff
• Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

[andrross]

Can you add details to the commit message about why this change was made originally and why it is safe to revert it without impacting any functionality?

[andrross]

I think we can omit a changelog entry since it doesn't change any user-facing functionality. I'll add the skip-changelog label to the PR.

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/6875/
• CommitID: 18f8b16
  Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[heemin32]

Can you add details to the commit message about why this change was made originally and why it is safe to revert it without impacting any functionality?

ACK

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/6876/
• CommitID: 8818409
  Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/6877/
• CommitID: f7d66aa
  Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: SUCCESS ✅
• URL: https://build.ci.opensearch.org/job/gradle-check/6878/
• CommitID: 3d891b9

[heemin32]

Test Result (2 failures / +2)
org.opensearch.repositories.s3.RepositoryS3ClientYamlTestSuiteIT.test {yaml=repository_s3/20_repository_permanent_credentials/Snapshot and Restore with repository-s3 using permanent credentials}
org.opensearch.repositories.s3.RepositoryS3ClientYamlTestSuiteIT.test {yaml=repository_s3/20_repository_permanent_credentials/Snapshot and Restore with repository-s3 using permanent credentials}

Related issue: https://build.ci.opensearch.org/job/gradle-check/6880/console

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: SUCCESS ✅
• URL: https://build.ci.opensearch.org/job/gradle-check/6881/
• CommitID: 7bc1856

[dblock]

@heemin32 Do you have an issue to re-add this code without the performance penalty? notably the check that raises [geo_point] field type does not accept more than 3 values looked like a useful feature.

[heemin32]

@dblock It does check and raise exception. One difference is exception message itself. One is field type does not accept > 3 dimensions and the other is [geo_point] field type does not accept more than 3 values.
We cannot provide specific type name here because xy_point also uses same code path.

One option is to let PointParser to have array parser and move the logic from PointParser to the array parser. Not sure it is worth to do though.

public PointParser(
            String field,
            Supplier<P> pointSupplier,
            CheckedBiFunction<XContentParser, P, P, IOException> objectParser,
            P nullValue,
            boolean ignoreZValue,
            boolean ignoreMalformed,
            CheckedBiFunction<XContentParser, P, P, IOException> arrayParser,
        ) {
            this.field = field;
            this.pointSupplier = pointSupplier;
            this.objectParser = objectParser;
            this.nullValue = nullValue;
            this.ignoreZValue = ignoreZValue;
            this.ignoreMalformed = ignoreMalformed;
            this.geometryParser = new GeometryParser(true, true, true);
            this.arrayParser = arrayParser;
        }

[heemin32]

@dblock Could you create backport 2.x PR?

[dblock]

[Backport 2.x] Revert changes in AbstractPointGeometryFieldMapper #5260

That was #5260 already I believe.