Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored the GeoBoundsAggregation code from server folder to geo module. #3992

Merged
merged 1 commit into from
Aug 9, 2022

Conversation

navneet1v
Copy link
Contributor

@navneet1v navneet1v commented Jul 22, 2022

Description

This change refactors the code of GeoBoundsAggregation on GeoPoints from the server folder to the modules/geo.

The proposal is being discussed and signed off here: opensearch-project/geospatial#92

Issues Resolved

#3980

High Level Task: opensearch-project/geospatial#104

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

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.

Signed-off-by: Navneet Verma navneev@amazon.com

@navneet1v navneet1v requested review from a team and reta as code owners July 22, 2022 20:54
@navneet1v navneet1v added Plugins Search:Aggregations v2.5.0 'Issues and PRs related to version v2.5.0' labels Jul 22, 2022
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@navneet1v navneet1v force-pushed the main branch 2 times, most recently from 2bdc39d to 38b4847 Compare July 22, 2022 21:05
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Copy link
Collaborator

@nknize nknize left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this! I have just a couple initial questions before fully approving.

*/
@Override
protected boolean addMockGeoShapeFieldMapper() {
return false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what test scenarios would this get set to true?

Copy link
Contributor Author

@navneet1v navneet1v Jul 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per the documentation there are test cases which are written in the server folder which depends on this, which test the indexing of geoshape as the geoshape mapper is still in the server folder.

Here is the failure CI link which suggest what tests failed when we removed this addMockGeoShapeFieldMapper function.

https://build.ci.opensearch.org/job/gradle-check/1049/console

REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.search.geo.LegacyGeoShapeIntegrationIT.testIgnoreMalformed" -Dtests.seed=451EB605A707A8A9 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=is-IS -Dtests.timezone=America/Argentina/Salta -Druntime.java=17

org.opensearch.search.geo.LegacyGeoShapeIntegrationIT > testIgnoreMalformed FAILED
    MapperParsingException[Failed to parse mapping [_doc]: No handler for type [geo_shape] declared on field [shape]]; nested: MapperParsingException[No handler for type [geo_shape] declared on field [shape]];
        at __randomizedtesting.SeedInfo.seed([451EB605A707A8A9:2383346D65B64520]:0)
        at org.opensearch.index.mapper.MapperService.internalMerge(MapperService.java:443)
        at org.opensearch.index.mapper.MapperService.merge(MapperService.java:411)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.updateIndexMappingsAndBuildSortOrder(MetadataCreateIndexService.java:1115)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.lambda$applyCreateIndexWithTemporaryService$2(MetadataCreateIndexService.java:453)
        at org.opensearch.indices.IndicesService.withTempIndexService(IndicesService.java:687)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexWithTemporaryService(MetadataCreateIndexService.java:451)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequestWithV1Templates(MetadataCreateIndexService.java:556)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequest(MetadataCreateIndexService.java:413)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequest(MetadataCreateIndexService.java:420)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService$1.execute(MetadataCreateIndexService.java:326)
        at org.opensearch.cluster.ClusterStateUpdateTask.execute(ClusterStateUpdateTask.java:65)
        at org.opensearch.cluster.service.MasterService.executeTasks(MasterService.java:824)
        at org.opensearch.cluster.service.MasterService.calculateTaskOutputs(MasterService.java:395)
        at org.opensearch.cluster.service.MasterService.runTasks(MasterService.java:266)
        at org.opensearch.cluster.service.MasterService$Batcher.run(MasterService.java:190)
        at org.opensearch.cluster.service.TaskBatcher.runIfNotProcessed(TaskBatcher.java:176)
        at org.opensearch.cluster.service.TaskBatcher$BatchedTask.run(TaskBatcher.java:214)
        at org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:739)
        at org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor$TieBreakingPrioritizedRunnable.runAndClean(PrioritizedOpenSearchThreadPoolExecutor.java:282)
        at org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedOpenSearchThreadPoolExecutor.java:245)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.lang.Thread.run(Thread.java:833)

        Caused by:
        MapperParsingException[No handler for type [geo_shape] declared on field [shape]]
            at org.opensearch.index.mapper.ObjectMapper$TypeParser.parseProperties(ObjectMapper.java:380)
            at org.opensearch.index.mapper.ObjectMapper$TypeParser.parseObjectOrDocumentTypeProperties(ObjectMapper.java:295)
            at org.opensearch.index.mapper.RootObjectMapper$TypeParser.parse(RootObjectMapper.java:182)
            at org.opensearch.index.mapper.DocumentMapperParser.parse(DocumentMapperParser.java:143)
            at org.opensearch.index.mapper.DocumentMapperParser.parse(DocumentMapperParser.java:132)
            at org.opensearch.index.mapper.MapperService.internalMerge(MapperService.java:441)
            ... 22 more

REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.search.geo.LegacyGeoShapeIntegrationIT.testOrientationPersistence" -Dtests.seed=451EB605A707A8A9 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=is-IS -Dtests.timezone=America/Argentina/Salta -Druntime.java=17

org.opensearch.search.geo.LegacyGeoShapeIntegrationIT > testOrientationPersistence FAILED
    MapperParsingException[Failed to parse mapping [_doc]: No handler for type [geo_shape] declared on field [location]]; nested: MapperParsingException[No handler for type [geo_shape] declared on field [location]];
        at __randomizedtesting.SeedInfo.seed([451EB605A707A8A9:14C1668D19C14D41]:0)
        at org.opensearch.index.mapper.MapperService.internalMerge(MapperService.java:443)
        at org.opensearch.index.mapper.MapperService.merge(MapperService.java:411)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.updateIndexMappingsAndBuildSortOrder(MetadataCreateIndexService.java:1115)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.lambda$applyCreateIndexWithTemporaryService$2(MetadataCreateIndexService.java:453)
        at org.opensearch.indices.IndicesService.withTempIndexService(IndicesService.java:687)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexWithTemporaryService(MetadataCreateIndexService.java:451)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequestWithV1Templates(MetadataCreateIndexService.java:556)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequest(MetadataCreateIndexService.java:413)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequest(MetadataCreateIndexService.java:420)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService$1.execute(MetadataCreateIndexService.java:326)
        at org.opensearch.cluster.ClusterStateUpdateTask.execute(ClusterStateUpdateTask.java:65)
        at org.opensearch.cluster.service.MasterService.executeTasks(MasterService.java:824)
        at org.opensearch.cluster.service.MasterService.calculateTaskOutputs(MasterService.java:395)
        at org.opensearch.cluster.service.MasterService.runTasks(MasterService.java:266)
        at org.opensearch.cluster.service.MasterService$Batcher.run(MasterService.java:190)
        at org.opensearch.cluster.service.TaskBatcher.runIfNotProcessed(TaskBatcher.java:176)
        at org.opensearch.cluster.service.TaskBatcher$BatchedTask.run(TaskBatcher.java:214)
        at org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:739)
        at org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor$TieBreakingPrioritizedRunnable.runAndClean(PrioritizedOpenSearchThreadPoolExecutor.java:282)
        at org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedOpenSearchThreadPoolExecutor.java:245)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.lang.Thread.run(Thread.java:833)

        Caused by:
        MapperParsingException[No handler for type [geo_shape] declared on field [location]]
            at org.opensearch.index.mapper.ObjectMapper$TypeParser.parseProperties(ObjectMapper.java:380)
            at org.opensearch.index.mapper.ObjectMapper$TypeParser.parseObjectOrDocumentTypeProperties(ObjectMapper.java:295)
            at org.opensearch.index.mapper.RootObjectMapper$TypeParser.parse(RootObjectMapper.java:182)
            at org.opensearch.index.mapper.DocumentMapperParser.parse(DocumentMapperParser.java:143)
            at org.opensearch.index.mapper.DocumentMapperParser.parse(DocumentMapperParser.java:132)
            at org.opensearch.index.mapper.MapperService.internalMerge(MapperService.java:441)
            ... 22 more

REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.search.geo.LegacyGeoShapeIntegrationIT.testDisallowExpensiveQueries" -Dtests.seed=451EB605A707A8A9 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=is-IS -Dtests.timezone=America/Argentina/Salta -Druntime.java=17

org.opensearch.search.geo.LegacyGeoShapeIntegrationIT > testDisallowExpensiveQueries FAILED
    MapperParsingException[Failed to parse mapping [_doc]: No handler for type [geo_shape] declared on field [shape]]; nested: MapperParsingException[No handler for type [geo_shape] declared on field [shape]];
        at __randomizedtesting.SeedInfo.seed([451EB605A707A8A9:6209CE80B3E38F79]:0)
        at org.opensearch.index.mapper.MapperService.internalMerge(MapperService.java:443)
        at org.opensearch.index.mapper.MapperService.merge(MapperService.java:411)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.updateIndexMappingsAndBuildSortOrder(MetadataCreateIndexService.java:1115)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.lambda$applyCreateIndexWithTemporaryService$2(MetadataCreateIndexService.java:453)
        at org.opensearch.indices.IndicesService.withTempIndexService(IndicesService.java:687)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexWithTemporaryService(MetadataCreateIndexService.java:451)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequestWithV1Templates(MetadataCreateIndexService.java:556)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequest(MetadataCreateIndexService.java:413)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequest(MetadataCreateIndexService.java:420)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService$1.execute(MetadataCreateIndexService.java:326)
        at org.opensearch.cluster.ClusterStateUpdateTask.execute(ClusterStateUpdateTask.java:65)
        at org.opensearch.cluster.service.MasterService.executeTasks(MasterService.java:824)
        at org.opensearch.cluster.service.MasterService.calculateTaskOutputs(MasterService.java:395)
        at org.opensearch.cluster.service.MasterService.runTasks(MasterService.java:266)
        at org.opensearch.cluster.service.MasterService$Batcher.run(MasterService.java:190)
        at org.opensearch.cluster.service.TaskBatcher.runIfNotProcessed(TaskBatcher.java:176)
        at org.opensearch.cluster.service.TaskBatcher$BatchedTask.run(TaskBatcher.java:214)
        at org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:739)
        at org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor$TieBreakingPrioritizedRunnable.runAndClean(PrioritizedOpenSearchThreadPoolExecutor.java:282)
        at org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedOpenSearchThreadPoolExecutor.java:245)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.lang.Thread.run(Thread.java:833)

        Caused by:
        MapperParsingException[No handler for type [geo_shape] declared on field [shape]]
            at org.opensearch.index.mapper.ObjectMapper$TypeParser.parseProperties(ObjectMapper.java:380)
            at org.opensearch.index.mapper.ObjectMapper$TypeParser.parseObjectOrDocumentTypeProperties(ObjectMapper.java:295)
            at org.opensearch.index.mapper.RootObjectMapper$TypeParser.parse(RootObjectMapper.java:182)
            at org.opensearch.index.mapper.DocumentMapperParser.parse(DocumentMapperParser.java:143)
            at org.opensearch.index.mapper.DocumentMapperParser.parse(DocumentMapperParser.java:132)
            at org.opensearch.index.mapper.MapperService.internalMerge(MapperService.java:441)
            ... 22 more

REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.search.geo.LegacyGeoShapeIntegrationIT.testLegacyCircle" -Dtests.seed=451EB605A707A8A9 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=is-IS -Dtests.timezone=America/Argentina/Salta -Druntime.java=17

org.opensearch.search.geo.LegacyGeoShapeIntegrationIT > testLegacyCircle FAILED
    MapperParsingException[Failed to parse mapping [_doc]: No handler for type [geo_shape] declared on field [shape]]; nested: MapperParsingException[No handler for type [geo_shape] declared on field [shape]];
        at __randomizedtesting.SeedInfo.seed([451EB605A707A8A9:5646E7CB6FDF1CF8]:0)
        at org.opensearch.index.mapper.MapperService.internalMerge(MapperService.java:443)
        at org.opensearch.index.mapper.MapperService.merge(MapperService.java:411)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.updateIndexMappingsAndBuildSortOrder(MetadataCreateIndexService.java:1115)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.lambda$applyCreateIndexWithTemporaryService$2(MetadataCreateIndexService.java:453)
        at org.opensearch.indices.IndicesService.withTempIndexService(IndicesService.java:687)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexWithTemporaryService(MetadataCreateIndexService.java:451)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequestWithV1Templates(MetadataCreateIndexService.java:556)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequest(MetadataCreateIndexService.java:413)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequest(MetadataCreateIndexService.java:420)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService$1.execute(MetadataCreateIndexService.java:326)
        at org.opensearch.cluster.ClusterStateUpdateTask.execute(ClusterStateUpdateTask.java:65)
        at org.opensearch.cluster.service.MasterService.executeTasks(MasterService.java:824)
        at org.opensearch.cluster.service.MasterService.calculateTaskOutputs(MasterService.java:395)
        at org.opensearch.cluster.service.MasterService.runTasks(MasterService.java:266)
        at org.opensearch.cluster.service.MasterService$Batcher.run(MasterService.java:190)
        at org.opensearch.cluster.service.TaskBatcher.runIfNotProcessed(TaskBatcher.java:176)
        at org.opensearch.cluster.service.TaskBatcher$BatchedTask.run(TaskBatcher.java:214)
        at org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:739)
        at org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor$TieBreakingPrioritizedRunnable.runAndClean(PrioritizedOpenSearchThreadPoolExecutor.java:282)
        at org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedOpenSearchThreadPoolExecutor.java:245)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.lang.Thread.run(Thread.java:833)

        Caused by:
        MapperParsingException[No handler for type [geo_shape] declared on field [shape]]
            at org.opensearch.index.mapper.ObjectMapper$TypeParser.parseProperties(ObjectMapper.java:380)
            at org.opensearch.index.mapper.ObjectMapper$TypeParser.parseObjectOrDocumentTypeProperties(ObjectMapper.java:295)
            at org.opensearch.index.mapper.RootObjectMapper$TypeParser.parse(RootObjectMapper.java:182)
            at org.opensearch.index.mapper.DocumentMapperParser.parse(DocumentMapperParser.java:143)
            at org.opensearch.index.mapper.DocumentMapperParser.parse(DocumentMapperParser.java:132)
            at org.opensearch.index.mapper.MapperService.internalMerge(MapperService.java:441)
            ... 22 more

REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.search.geo.LegacyGeoShapeIntegrationIT.testIndexShapeRouting" -Dtests.seed=451EB605A707A8A9 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=is-IS -Dtests.timezone=America/Argentina/Salta -Druntime.java=17

org.opensearch.search.geo.LegacyGeoShapeIntegrationIT > testIndexShapeRouting FAILED
    MapperParsingException[Failed to parse mapping [_doc]: No handler for type [geo_shape] declared on field [shape]]; nested: MapperParsingException[No handler for type [geo_shape] declared on field [shape]];
        at __randomizedtesting.SeedInfo.seed([451EB605A707A8A9:12DFAF8F1ED154AC]:0)
        at org.opensearch.index.mapper.MapperService.internalMerge(MapperService.java:443)
        at org.opensearch.index.mapper.MapperService.merge(MapperService.java:411)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.updateIndexMappingsAndBuildSortOrder(MetadataCreateIndexService.java:1115)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.lambda$applyCreateIndexWithTemporaryService$2(MetadataCreateIndexService.java:453)
        at org.opensearch.indices.IndicesService.withTempIndexService(IndicesService.java:687)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexWithTemporaryService(MetadataCreateIndexService.java:451)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequestWithV1Templates(MetadataCreateIndexService.java:556)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequest(MetadataCreateIndexService.java:413)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService.applyCreateIndexRequest(MetadataCreateIndexService.java:420)
        at org.opensearch.cluster.metadata.MetadataCreateIndexService$1.execute(MetadataCreateIndexService.java:326)
        at org.opensearch.cluster.ClusterStateUpdateTask.execute(ClusterStateUpdateTask.java:65)
        at org.opensearch.cluster.service.MasterService.executeTasks(MasterService.java:824)
        at org.opensearch.cluster.service.MasterService.calculateTaskOutputs(MasterService.java:395)
        at org.opensearch.cluster.service.MasterService.runTasks(MasterService.java:266)
        at org.opensearch.cluster.service.MasterService$Batcher.run(MasterService.java:190)
        at org.opensearch.cluster.service.TaskBatcher.runIfNotProcessed(TaskBatcher.java:176)
        at org.opensearch.cluster.service.TaskBatcher$BatchedTask.run(TaskBatcher.java:214)
        at org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:739)
        at org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor$TieBreakingPrioritizedRunnable.runAndClean(PrioritizedOpenSearchThreadPoolExecutor.java:282)
        at org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedOpenSearchThreadPoolExecutor.java:245)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.lang.Thread.run(Thread.java:833)

        Caused by:
        MapperParsingException[No handler for type [geo_shape] declared on field [shape]]
            at org.opensearch.index.mapper.ObjectMapper$TypeParser.parseProperties(ObjectMapper.java:380)
            at org.opensearch.index.mapper.ObjectMapper$TypeParser.parseObjectOrDocumentTypeProperties(ObjectMapper.java:295)
            at org.opensearch.index.mapper.RootObjectMapper$TypeParser.parse(RootObjectMapper.java:182)
            at org.opensearch.index.mapper.DocumentMapperParser.parse(DocumentMapperParser.java:143)
            at org.opensearch.index.mapper.DocumentMapperParser.parse(DocumentMapperParser.java:132)
            at org.opensearch.index.mapper.MapperService.internalMerge(MapperService.java:441)
            ... 22 more

DoubleArray posRights;
DoubleArray negLefts;
DoubleArray negRights;
final class GeoBoundsAggregator extends AbstractGeoBoundsAggregator<ValuesSource.GeoPoint> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will there be a new ValuesSource.GeoShape when shape docvalues are added to lucene? Or are you planning on creating a ValuesSource.Geo that handles both?

Copy link
Contributor Author

@navneet1v navneet1v Jul 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was planning to create a new ValueSource which will be ValueSource.GeoShape for geoshapes.

On your comment if I am planning to combine them the honest ans is no I have not thought about it for now. I am not even sure if it is possible or not. But in regards for this PR, I would like to keep this as a refactor code PR.

I will try to put forward proposal on combining the ValuesSource.GeoShape and ValuesSource.GeoPoint to make ValuesSource.Geo and then we can take it from there.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@VijayanB
Copy link
Member

VijayanB commented Jul 30, 2022

@navneet1v Since you are refactoring/moving one aggregation at a time, shall we create tasks for list of aggregation that will be refactored and link to corresponding PR?

@navneet1v
Copy link
Contributor Author

@navneet1v Since you are refactoring/moving one aggregation at a time, shall we create tasks for list of aggregation that will be refactored and link to corresponding PR?

Sure. I think I had this thing in mind but I forgot. Let me create an issue and link it in all the places.

The issue will be an uber issue which will list RFCs, and implementation issues for every Aggregation.

@navneet1v
Copy link
Contributor Author

@navneet1v Since you are refactoring/moving one aggregation at a time, shall we create tasks for list of aggregation that will be refactored and link to corresponding PR?

Sure. I think I had this thing in mind but I forgot. Let me create an issue and link it in all the places.

The issue will be an uber issue which will list RFCs, and implementation issues for every Aggregation.

Issue: opensearch-project/geospatial#104

@navneet1v navneet1v force-pushed the main branch 2 times, most recently from c50c38f to c8f4681 Compare August 1, 2022 23:37
@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2022

Gradle Check (Jenkins) Run Completed with:

@navneet1v navneet1v requested a review from a team August 3, 2022 01:27
@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2022

Gradle Check (Jenkins) Run Completed with:

@navneet1v
Copy link
Contributor Author

@opensearch-project/opensearch-core can anyone take a look into this CR, it has been there for long period of time.

…server folder to geo module.(opensearch-project#3980)

Signed-off-by: Navneet Verma <navneev@amazon.com>
@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2022

Gradle Check (Jenkins) Run Completed with:

Copy link
Collaborator

@nknize nknize left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! One minor nit we can do in a follow up PR.

@navneet1v
Copy link
Contributor Author

LGTM! One minor nit we can do in a follow up PR.

sure will fix in the next PR for moving the GeoTile and GeoHashGird aggregation to Geo module

@nknize nknize merged commit 4751f3b into opensearch-project:main Aug 9, 2022
@navneet1v navneet1v added the backport 2.x Backport to 2.x branch label Aug 9, 2022
@navneet1v
Copy link
Contributor Author

Backport of 2.x is pending. Need to figure out a way to backport these changes to the 2.x branch.

@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-3992-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 4751f3bd296e6952b852a457a82220ec248b037a
# Push it to GitHub
git push --set-upstream origin backport/backport-3992-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-3992-to-2.x.

@navneet1v
Copy link
Contributor Author

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

BASH (SHELL) (16 lines)

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-3992-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 4751f3bd296e6952b852a457a82220ec248b037a
# Push it to GitHub
git push --set-upstream origin backport/backport-3992-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-3992-to-2.x.

Back-porting PR: #4179

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants