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

Fix validate_* merge policy for GeoPointFieldMapper #10165

Merged
merged 1 commit into from Mar 24, 2015

Conversation

nknize
Copy link
Contributor

@nknize nknize commented Mar 19, 2015

Fail merge if validate_lat or validate_lon values are not equal. This will prevent inconsistencies between geo_points in a merged index, and parse exceptions for bounding_box and distance filters.

Also merged separate GeoPoint test classes into a single GeoPointFieldMapperTest to be consistent with GeoShapeFieldMapperTests.

closes #10164

@nknize nknize added >bug v2.0.0-beta1 good first issue low hanging fruit v1.5.0 :Analytics/Geo Indexing, search aggregations of geo points and shapes review labels Mar 19, 2015
@s1monw
Copy link
Contributor

s1monw commented Mar 20, 2015

@rjernst can you take a look at this please?

DocumentMapper stage1 = parser.parse(stage1Mapping);
String stage2Mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true).field("geohash", true)
.field("validate", false).field("normalize", false).endObject().endObject()
Copy link
Member

Choose a reason for hiding this comment

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

why change normalize to false here? aren't you just trying to check validate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct. normalize will be changing in a separate PR. I must have accidentally included it in this one. :) Made changes.

@rjernst
Copy link
Member

rjernst commented Mar 20, 2015

LGTM, just two tiny comments. Thanks for merging these test suites!

@martijnvg martijnvg added v1.5.1 and removed v1.5.0 labels Mar 23, 2015
@nknize nknize force-pushed the fix/10164 branch 2 times, most recently from c5a6e6f to cde3f02 Compare March 23, 2015 20:31
Fail merge if validate_lat or validate_lon values are not equal. This will prevent inconsistencies between geo_points in a merged index, and parse exceptions for bounding_box and distance filters.

Also merged separate GeoPoint test classes into a single GeoPointFieldMapperTest to be consistent with GeoShapeFieldMapperTests.

closes elastic#10164
@nknize nknize merged commit e43f3a9 into elastic:master Mar 24, 2015
@clintongormley clintongormley changed the title [GEO] Fix validate_* merge policy for GeoPointFieldMapper Fix validate_* merge policy for GeoPointFieldMapper Jun 7, 2015
@nknize nknize deleted the fix/10164 branch May 27, 2016 16:23
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 >bug good first issue low hanging fruit v1.5.1 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GEO] GeoPointFieldMapper.validate_* overwritten on merge
5 participants