Fix fields API for geo_point fields inside other arrays #99868
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cbuescher
changed the title
cbuescher
force-pushed
the
fix-99781
branch
from
64851d1
to
42f403b
Compare
The fields API currently doesn't work for geopoint arrays inside outer objects that again are expressed as multi-values arrays. The reason is that upon extracting the leaf values from source (i.e via Source#extractValue) we accumulate intermediate array structures as additinal lists in the source value we then try to parse. For other field types, especially those that use SourceValueFetcher, this doesn't matter because we have a list-flattening step in SourceValueFetcher#fetchValues, but for geo_point types the parser cannot deal with the additional list levels. This change modifies the source extraction so that we avoid nesting to many list structures inside each other, which I believe currently is wrong but is masked by the list deduplication mentioned above. Closes elastic#99781
cbuescher
force-pushed
the
fix-99781
branch
from
42f403b
to
bb1ee8a
Compare
cbuescher
changed the title
cbuescher
added
:Search/Search
|
Hi @cbuescher, I've created a changelog YAML for you. |
|
Pinging @elastic/es-search (Team:Search) |
romseygeek
reviewed
Sep 26, 2023
server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java
Outdated
Show resolved
Hide resolved
|
@romseygeek thanks for the review, I made the changes to the tests you suggested. |
romseygeek
approved these changes
Sep 26, 2023
server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java
Outdated
Show resolved
Hide resolved
…ieldFetcherTests.java Co-authored-by: Alan Woodward <romseygeek@gmail.com>
…ieldFetcherTests.java Co-authored-by: Alan Woodward <romseygeek@gmail.com>
|
@elasticmachine run elasticsearch-ci/bwc |
cbuescher
added
the
auto-backport-and-merge
💚 Backport successful
|
cbuescher
added a commit
to cbuescher/elasticsearch
that referenced
this pull request
Sep 26, 2023
The fields API currently doesn't work for geopoint arrays inside outer objects that again are expressed as multi-values arrays. The reason is that upon extracting the leaf values from source (i.e via Source#extractValue) we accumulate intermediate array structures as additinal lists in the source value we then try to parse. For other field types, especially those that use SourceValueFetcher, this doesn't matter because we have a list-flattening step in SourceValueFetcher#fetchValues, but for geo_point types the parser cannot deal with the additional list levels. This change modifies the source extraction so that we avoid nesting to many list structures inside each other, which I believe currently is wrong but is masked by the list deduplication mentioned above. Closes elastic#99781
elasticsearchmachine
pushed a commit
that referenced
this pull request
Sep 26, 2023
) The fields API currently doesn't work for geopoint arrays inside outer objects that again are expressed as multi-values arrays. The reason is that upon extracting the leaf values from source (i.e via Source#extractValue) we accumulate intermediate array structures as additinal lists in the source value we then try to parse. For other field types, especially those that use SourceValueFetcher, this doesn't matter because we have a list-flattening step in SourceValueFetcher#fetchValues, but for geo_point types the parser cannot deal with the additional list levels. This change modifies the source extraction so that we avoid nesting to many list structures inside each other, which I believe currently is wrong but is masked by the list deduplication mentioned above. Closes #99781
|
Thank you @cbuescher. |
piergm
pushed a commit
to piergm/elasticsearch
that referenced
this pull request
Oct 2, 2023
The fields API currently doesn't work for geopoint arrays inside outer objects that again are expressed as multi-values arrays. The reason is that upon extracting the leaf values from source (i.e via Source#extractValue) we accumulate intermediate array structures as additinal lists in the source value we then try to parse. For other field types, especially those that use SourceValueFetcher, this doesn't matter because we have a list-flattening step in SourceValueFetcher#fetchValues, but for geo_point types the parser cannot deal with the additional list levels. This change modifies the source extraction so that we avoid nesting to many list structures inside each other, which I believe currently is wrong but is masked by the list deduplication mentioned above. Closes elastic#99781
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The fields API currently doesn't work for geopoint arrays inside outer objects
that again are expressed as multi-values arrays. The reason is that upon
extracting the leaf values from source (i.e via Source#extractValue) we
accumulate intermediate array structures as additinal lists in the source value
we then try to parse. For other field types, especially those that use
SourceValueFetcher, this doesn't matter because we have a list-flattening step
in SourceValueFetcher#fetchValues, but for geo_point types the parser cannot
deal with the additional list levels. This change modifies the source extraction
so that we avoid nesting to many list structures inside each other, which I
believe currently is wrong but is masked by the list deduplication mentioned
above.
Closes #99781