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

[Geospatial] Add new geo shape filter field to support geospatial search query #3605

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

VijayanB
Copy link
Member

@VijayanB VijayanB commented Mar 14, 2023

Description

Add new filter query, 'geo_shape' to search geospatial field types . This is required as part of Maps plugin which
uses geo_shape filter query to search documents.

Issues Resolved

opensearch-project/dashboards-maps#213

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

@VijayanB VijayanB requested a review from a team as a code owner March 14, 2023 01:10
@VijayanB VijayanB force-pushed the add-geo-shape-filter branch 3 times, most recently from aa02477 to 50a6879 Compare March 14, 2023 01:40
@codecov-commenter
Copy link

codecov-commenter commented Mar 14, 2023

Codecov Report

Merging #3605 (5ac7d4e) into main (f8c9182) will decrease coverage by 0.06%.
The diff coverage is 75.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #3605      +/-   ##
==========================================
- Coverage   66.47%   66.42%   -0.06%     
==========================================
  Files        3209     3210       +1     
  Lines       61617    61625       +8     
  Branches     9504     9506       +2     
==========================================
- Hits        40958    40932      -26     
- Misses      18386    18414      +28     
- Partials     2273     2279       +6     
Flag Coverage Δ
Linux ?
Windows 66.42% <75.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ommon/opensearch_query/filters/get_filter_field.ts 33.33% <0.00%> (-4.17%) ⬇️
...gins/data/common/opensearch_query/filters/index.ts 100.00% <ø> (ø)
...ommon/opensearch_query/filters/geo_shape_filter.ts 100.00% <100.00%> (ø)

... and 6 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@VijayanB VijayanB marked this pull request as draft March 14, 2023 16:47
@VijayanB VijayanB force-pushed the add-geo-shape-filter branch 2 times, most recently from b80d682 to 243d2a6 Compare March 14, 2023 17:09
@VijayanB VijayanB marked this pull request as ready for review March 14, 2023 17:36
@VijayanB VijayanB force-pushed the add-geo-shape-filter branch 2 times, most recently from be85917 to f159296 Compare March 14, 2023 22:08
@VijayanB VijayanB requested a review from AMoo-Miki March 14, 2023 22:13
@VijayanB
Copy link
Member Author

@AMoo-Miki Can you take a look at PR one more time? Thanks. Sorry for the rush, since we will be building feature in Maps using this PR and this is blocker for our current development.

@kavilla
Copy link
Member

kavilla commented Mar 16, 2023

@VijayanB for speed could you provide a screen shot or describe recreation steps how to be able to use this new functionality.


export type GeoShapeFilter = Filter & {
meta: GeoShapeFilterMeta;
geo_shape: any;
Copy link
Member

Choose a reason for hiding this comment

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

Will we consider define a more specific interface or type for the geo_shape property?

Copy link
Member Author

Choose a reason for hiding this comment

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

geo_shape will have dynamic field name and "ignored_unmapped" as well. I couldn't find a way to define both in same structure. Do you know how to do that?

Copy link
Member

Choose a reason for hiding this comment

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

Not quite sure about what should be geo_shape, but it is similar to meta, will this work?

export type GeoShapeFilter = Filter & {
  meta: GeoShapeFilterMeta;
  geo_shape: {
    [fieldName: string]: {
      shape?: ShapeFilter;
      indexed_shape?: PreIndexedShapeFilter;
      relation?: GeoShapeRelation;
      ignore_unmapped?: boolean;
    };
  };
};

Copy link
Member

Choose a reason for hiding this comment

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

Not a blocker. I will just approve it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here ignore_unmapped should be inside geo_shape , something like below

geo_shape: {
    [fieldName: string]: {
      shape?: ShapeFilter;
      indexed_shape?: PreIndexedShapeFilter;
      relation?: GeoShapeRelation;
    },
   ignore_unmapped?: boolean;
}

but typescript don't like this, since ignore_unmapped is also string and it can't differentiate whether is it part of fieldName or ignore_mapped.

@VijayanB
Copy link
Member Author

VijayanB commented Mar 17, 2023

@VijayanB for speed could you provide a screen shot or describe recreation steps how to be able to use this new functionality.

Sure. geo_shape is a query type that can be used to define search query for geospatial index. Currently Region Maps and Coordinate maps uses geo_polygon query type, but, it is deprecated and not encourage any more. Hence, this PR, will understand any query with Filter of type GeoShape and can serialize into geo_shape query for search request .

Maps will be using geo_shape filter instead of geo_polygon.

ananzh
ananzh previously approved these changes Mar 17, 2023
ananzh
ananzh previously approved these changes Mar 21, 2023
@VijayanB
Copy link
Member Author

@kavilla @AMoo-Miki Can you please take a look at this PR?

Add new filter query, 'geo_shape' to search geospatial
field types . This new filter query will replace geo_polygon
query later. With this fiter, dashboard can perform spatial
relationssips with shape or predefined shape from an index
against an index.

Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>
@VijayanB VijayanB changed the title Add geo shape filter field [Geospatial] Add new geo shape filter field to support geospatial search query Mar 21, 2023
@VijayanB VijayanB requested review from ananzh and AMoo-Miki and removed request for AMoo-Miki and ananzh March 21, 2023 17:10
@abbyhu2000 abbyhu2000 merged commit 71eb3f2 into opensearch-project:main Mar 23, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 23, 2023
Add new filter query, 'geo_shape' to search geospatial
field types . This new filter query will replace geo_polygon
query later. With this fiter, dashboard can perform spatial
relationssips with shape or predefined shape from an index
against an index.

Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>
(cherry picked from commit 71eb3f2)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
ananzh added a commit that referenced this pull request Mar 24, 2023
… geospatial search query (#3681)

* Add geo shape filter field to supported filter formats (#3605)

Add new filter query, 'geo_shape' to search geospatial
field types . This new filter query will replace geo_polygon
query later. With this fiter, dashboard can perform spatial
relationssips with shape or predefined shape from an index
against an index.

Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>
(cherry picked from commit 71eb3f2)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
Co-authored-by: Anan Zhuang <ananzh@amazon.com>
pjfitzgibbons pushed a commit to pjfitzgibbons/OpenSearch-Dashboards that referenced this pull request Mar 24, 2023
…oject#3605)

Add new filter query, 'geo_shape' to search geospatial
field types . This new filter query will replace geo_polygon
query later. With this fiter, dashboard can perform spatial
relationssips with shape or predefined shape from an index
against an index.

Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>
sikhote pushed a commit to sikhote/OpenSearch-Dashboards that referenced this pull request Apr 24, 2023
…oject#3605)

Add new filter query, 'geo_shape' to search geospatial
field types . This new filter query will replace geo_polygon
query later. With this fiter, dashboard can perform spatial
relationssips with shape or predefined shape from an index
against an index.

Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>
Signed-off-by: David Sinclair <david@sinclair.tech>
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.

6 participants