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 mvt error when returning partial results #98765

Merged
merged 7 commits into from
Aug 23, 2023

Conversation

iverase
Copy link
Contributor

@iverase iverase commented Aug 23, 2023

In #97619, we make stricter the parsing of the query result and we throw an error when we cannot add part of the result to the vector tile. This had an undesirable side effect because the flattened method used to generate tags for the vector tile keeps the original maps.

This PR adds the logic to be able to ignore map objects when adding properties to the tile

fixes #98730

@iverase iverase added >bug :Analytics/Geo Indexing, search aggregations of geo points and shapes v8.11.0 v8.10.1 labels Aug 23, 2023
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

Hi @iverase, I've created a changelog YAML for you.

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Aug 23, 2023
@iverase iverase added the v8.9.2 label Aug 23, 2023
Copy link
Contributor

@craigtaverner craigtaverner left a comment

Choose a reason for hiding this comment

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

LGTM, although I wonder why Maps.flatten does not do the job

@@ -64,6 +64,10 @@ public static void addPropertyToFeature(VectorTile.Tile.Feature.Builder feature,
// guard for null values
return;
}
if (value instanceof Map<?, ?> map) {
// maps should have been flattened already in Maps#flatten but still contains the original maps
Copy link
Contributor

Choose a reason for hiding this comment

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

Maps.flatten does include recursive code for this, so it is surprising it does not happen. Perhaps there is a bug in Maps.flatten.

Copy link
Contributor Author

@iverase iverase Aug 23, 2023

Choose a reason for hiding this comment

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

Maybe but it is used in other places and I thought it was too risky to mess with it. I will check and open an issue if necessary

@iverase iverase merged commit 5f43a15 into elastic:main Aug 23, 2023
12 checks passed
@iverase iverase deleted the fix_mvt_partial branch August 23, 2023 09:01
@iverase iverase added the auto-backport-and-merge Automatically create backport pull requests and merge when ready label Aug 23, 2023
iverase added a commit to iverase/elasticsearch that referenced this pull request Aug 23, 2023
This PR adds the logic to ignore map objects when adding properties to a tile
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.9
8.10

iverase added a commit to iverase/elasticsearch that referenced this pull request Aug 23, 2023
This PR adds the logic to ignore map objects when adding properties to a tile
elasticsearchmachine pushed a commit that referenced this pull request Aug 23, 2023
This PR adds the logic to ignore map objects when adding properties to a tile
elasticsearchmachine pushed a commit that referenced this pull request Aug 23, 2023
This PR adds the logic to ignore map objects when adding properties to a tile
dreamquster pushed a commit to dreamquster/elasticsearch that referenced this pull request Aug 26, 2023
This PR adds the logic to ignore map objects when adding properties to a tile
@JVerwolf JVerwolf removed the v8.10.1 label Aug 31, 2023
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 auto-backport-and-merge Automatically create backport pull requests and merge when ready >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.9.2 v8.10.0 v8.11.0
Projects
None yet
4 participants