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.

Sign up for GitHub

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

Jump to bottom

Re-enable support for array-valued geo_shape fields. #58786

Merged
merged 5 commits into from Jul 2, 2020
Merged

Re-enable support for array-valued geo_shape fields. #58786

merged 5 commits into from Jul 2, 2020

Conversation

talevy
Copy link
Contributor

@talevy talevy commented Jun 30, 2020

A regression in the mapping code led to geo_shape no longer supporting
array-valued fields. This commit fixes this support and adds an integration
test to make sure this problem does not return!

example:

PUT /example
{
    "mappings": {
        "properties": {
            "location": {
                "type": "geo_shape"
            }
        }
    }
}

PUT /example/_doc/1
{
    "location": [
        {
            "type": "LineString",
            "coordinates": [[100.0, 1.0], [101.0, 0.0]]
        },
        {
            "type": "LineString",
            "coordinates": [[100.0, 1.0], [101.0, 0.0]]
        }
    ]
}

original user issue posting: https://discuss.elastic.co/t/elastic-search-7-8-0-and-multiple-geoshapes/239350

@talevy
Re-enable support for array-valued geo_shape fields.
f9a976d 
A regression in the mapping code led to geo_shape no longer supporting
array-valued fields. This commit fixes this support and adds an integration
test to make sure this problem does not return!
@talevy talevy added >bug :Analytics/Geo Indexing, search aggregations of geo points and shapes Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v7.8.1 labels Jun 30, 2020
@talevy talevy requested a review from jtibshirani June 30, 2020 21:20
@elasticmachine
Copy link
Collaborator

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

jtibshirani
jtibshirani reviewed Jun 30, 2020
View reviewed changes
Copy link
Contributor

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

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

Thanks so much @talevy for fixing this. I totally should have noticed it in #58786!

server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java Outdated Show resolved Hide resolved
server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java Outdated Show resolved Hide resolved
@talevy
Copy link
Contributor Author

talevy commented Jul 1, 2020

note: points and geo-points have unit tests for arrays!

@talevy talevy added the v8.0.0 label Jul 1, 2020
iverase
iverase approved these changes Jul 2, 2020
View reviewed changes
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. just wondering if we should add a test in GeoShapeWithDocValuesFieldMapperTests and in LegacyGeoShapeFieldMapperTests

@talevy talevy merged commit 52ff121 into elastic:master Jul 2, 2020
@talevy talevy deleted the fixgeoshapearray branch July 2, 2020 16:39
talevy added a commit to talevy/elasticsearch that referenced this pull request Jul 2, 2020
@talevy
Re-enable support for array-valued geo_shape fields. (elastic#58786)
171e361 
A regression in the mapping code led to geo_shape no longer supporting
array-valued fields. This commit fixes this support and adds an integration
test to make sure this problem does not return!
@talevy talevy mentioned this pull request Jul 2, 2020
Merged
talevy added a commit that referenced this pull request Jul 2, 2020
@talevy
Re-enable support for array-valued geo_shape fields. (#58786) (#58943)
d516959 
A regression in the mapping code led to geo_shape no longer supporting
array-valued fields. This commit fixes this support and adds an integration
test to make sure this problem does not return!
talevy added a commit to talevy/elasticsearch that referenced this pull request Jul 2, 2020
@talevy
Re-enable support for array-valued geo_shape fields. (elastic#58786) (e…
acbbf27 
…lastic#58943)

A regression in the mapping code led to geo_shape no longer supporting
array-valued fields. This commit fixes this support and adds an integration
test to make sure this problem does not return!
@talevy talevy mentioned this pull request Jul 2, 2020
Merged
talevy added a commit that referenced this pull request Jul 2, 2020
@talevy
Re-enable support for array-valued geo_shape fields. (#58786) (#58943) (
18f3760 
#58954)

A regression in the mapping code led to geo_shape no longer supporting
array-valued fields. This commit fixes this support and adds an integration
test to make sure this problem does not return!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtibshirani jtibshirani jtibshirani left review comments

@iverase iverase iverase approved these changes

Assignees
No one assigned
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v7.8.1 v7.9.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@talevy @elasticmachine @jtibshirani @iverase @jakelandis