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

Refactor Spatial Field Mappers #55621

Merged
merged 6 commits into from
Apr 23, 2020

Conversation

nknize
Copy link
Contributor

@nknize nknize commented Apr 22, 2020

This PR refactors all spatial Field Mappers to a common AbstractGeometryFieldMapper that implements shared parameter functionality (e.g., ignore_malformed, ignore_z_value) and provides a common framework for overriding type parsing, and building in xpack. Common shape functionality is implemented in a new AbstractShapeGeometryFieldMapper that is reused and overridden in GeoShapeFieldMapper, GeoShapeFieldMapperWithDocValues, LegacyGeoShapeFieldMapper, and ShapeFieldMapper, respectively. This abstraction provides a reusable foundation for adding new xpack features; such as coordinate reference system support.

This commit refactors all spatial Field Mappers to a common AbstractGeometryFieldMapper that implements shared parameter functionality (e.g., ignore_malformed, ignore_z_value) and provides a common framework for overriding type parsing, and building in xpack. Common shape functionality is implemented in a new AbstractShapeGeometryFieldMapper that is reused and overridden in GeoShapeFieldMapper, GeoShapeFieldMapperWithDocValues, LegacyGeoShapeFieldMapper, and ShapeFieldMapper. This abstraction provides a reusable foundation for adding new xpack features; such as coordinate reference system support.
@nknize nknize added >non-issue :Analytics/Geo Indexing, search aggregations of geo points and shapes >refactoring v8.0.0 v7.8.0 labels Apr 22, 2020
@nknize nknize requested review from talevy and iverase April 22, 2020 19:34
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (:Analytics/Geo)

Copy link
Contributor

@iverase iverase left a comment

Choose a reason for hiding this comment

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

I love this change, it is awesome that all our geo fields inherit from the same subclass. I would like to explore the use of generics in AbstractGeometryFieldParser.TypeParser to avoid casting on the subclasses, otherwise +1

Copy link
Contributor

@talevy talevy left a comment

Choose a reason for hiding this comment

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

I like the hierarchy unification as well. thanks for that.

Copy link
Contributor

@iverase iverase left a comment

Choose a reason for hiding this comment

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

lgtm

@nknize nknize merged commit eb0b2c8 into elastic:master Apr 23, 2020
nknize added a commit to nknize/elasticsearch that referenced this pull request Apr 24, 2020
This commit refactors all spatial Field Mappers to a common
AbstractGeometryFieldMapper that implements shared parameter functionality
(e.g., ignore_malformed, ignore_z_value) and provides a common framework for
overriding type parsing, and building in xpack. Common shape functionality is
implemented in a new AbstractShapeGeometryFieldMapper that is reused and
overridden in GeoShapeFieldMapper, GeoShapeFieldMapperWithDocValues,
LegacyGeoShapeFieldMapper, and ShapeFieldMapper. This abstraction provides a
reusable foundation for adding new xpack features; such as coordinate reference
system support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes >non-issue >refactoring v7.8.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants