Optimize the repetitive code in the NumericType.rangeQuery method of the NumberFieldMapper class

[sunqijun1]

Description

I found that in the NumericType.rangeQuery, there is repetitive code in the calls for different types such as Long, Integer, double, etc., which affects readability. So I made optimizations to this part of the content, extracted the repetitive code, enhanced the readability of the code, and reduced code redundancy.

Related Issues

(https://github.com/opensearch-project/OpenSearch/pull/18530/files#r2158442607)

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.

[github-actions]

❌ Gradle check result for d80bd4c: 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 03161d0: 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 03161d0: 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 fed0682: 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 fed0682: 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 fed0682: 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?

[andrross]

@sunqijun1 I haven't reviewed the code change here but the backward compatibility tests that are failing are telling you this change breaks compatibility somehow. You can find the REPRODUCE WITH command in the build output that will give you a command to run the tests locally to help debug.

[sunqijun1]

@sunqijun1 I haven't reviewed the code change here but the backward compatibility tests that are failing are telling you this change breaks compatibility somehow. You can find the REPRODUCE WITH command in the build output that will give you a command to run the tests locally to help debug.

ok. I will check it

[github-actions]

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

@sunqijun1 I think your backward compatibility is breaking most likely because you are removing some public methods - you can verify the same as well if this is true.

The method you introduced public <T extends Number> Query createRangeQuery is probably not super-intuitive to use (for end consumers) compared to existing methods like doubleRangeQuery for example.
Also, exposing this method as a public method puts too much control to end-users - which might be disastrous as OpenSearch might not be able to handle type incompatible changes which a consumer might specify)
Plus, any consumer of this method will now have worry about 10 different parameters to pass which is the case with method you introduced.

I do like the idea of refactored code, but here is my proposed changes:

• Your new method createRangeQuery should be a private method/utility.
• Existing methods based on data type like doubleRangeQuery, longRangeQuery can just do a call to the above private method - and they should still exist. They are simple, intuitive, and only expose a limited control.

We can make the new method public if in future we identify the need to expose building a customized range query where a consumer specifies their min/max/etc., but, I don't agree on exposing it publicly now for sure.

Although with the refactored changes, introducing a new data-type will become far easier.

(ignore my approval earlier - was clicking Request Changes, but clicked on the wrong button)

[opensearch-trigger-bot]

This PR is stalled because it has been open for 30 days with no activity.

[sunqijun1]

你好，你的来信孙其君已收到，若有急事请电话联系

[opensearch-trigger-bot]

This PR is stalled because it has been open for 30 days with no activity.

[sunqijun1]

你好，你的来信孙其君已收到，若有急事请电话联系