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

[Maps] fix term join agg key collision #63324

Merged
merged 10 commits into from
Apr 17, 2020
Merged

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Apr 12, 2020

Fixes #63287

When terms joins where first implemented, the where clause did not exist so we thought name-spacing by right source index title would provide unique keys. This turned out to be an incorrect assumption. With where clause, it makes a lot of sense to have several joins from the same index pattern. A new key is needed to avoid naming collisions.

This PR fixes the name collision for join agg keys by using uuid from right source instead of index pattern title and term join field in the agg key.

@nreese nreese requested a review from a team as a code owner April 12, 2020 22:14
@nreese
Copy link
Contributor Author

nreese commented Apr 13, 2020

@elasticmachine merge upstream

@nreese nreese added release_note:fix [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v7.8.0 v8.0.0 labels Apr 13, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@nreese
Copy link
Contributor Author

nreese commented Apr 16, 2020

@elasticmachine merge upstream

Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

Thanks for addressing this technical debt. It was a poor decision initially to model it like that, and hid subtle bugs.

Some minor suggestions below.

x-pack/plugins/maps/common/constants.ts Outdated Show resolved Hide resolved
x-pack/plugins/maps/common/constants.ts Outdated Show resolved Hide resolved
@nreese
Copy link
Contributor Author

nreese commented Apr 16, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@nreese nreese merged commit 420ccff into elastic:master Apr 17, 2020
nreese added a commit to nreese/kibana that referenced this pull request Apr 17, 2020
* [Maps] fix term join agg key collision

* fix tslint and jest errors

* fix join functional test

* revert LayerDescriptor union and cast to VectorLayerDescriptor instead

* move getJoinKey out of constants and into its own file

Co-authored-by: Elastic Machine <[email protected]>
nreese added a commit that referenced this pull request Apr 17, 2020
* [Maps] fix term join agg key collision

* fix tslint and jest errors

* fix join functional test

* revert LayerDescriptor union and cast to VectorLayerDescriptor instead

* move getJoinKey out of constants and into its own file

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 17, 2020
* master: (40 commits)
  [APM]Upgrade apm-rum agent to latest version to fix full page reload (elastic#63723)
  add deprecation warning for legacy 3rd party plugins (elastic#62401)
  Migrate timelion vis (elastic#62819)
  Replacebad scope link with actual values (elastic#63444)
  Index pattern management UI -> TypeScript and New Platform Ready (create_index_pattern_wizard) (elastic#63111)
  [SIEM] Threat hunting enhancements: Filter for/out value, Show top field, Copy to Clipboard, Draggable chart legends (elastic#61207)
  [Maps] fix term join agg key collision (elastic#63324)
  [Ingest] Fix agent config key sorting (elastic#63488)
  [Monitoring] Fixed server response errors (elastic#63181)
  update elastic charts to 18.3.0 (elastic#63732)
  Start services (elastic#63720)
  [APM] Encode spaces when creating ML job (elastic#63683)
  Uptime 7.7 docs (elastic#62228)
  [DOCS] Updates remote cluster and ccr docs (elastic#63517)
  [Maps] Add 3rd party vector tile support (elastic#62084)
  [Endpoint][EPM] Retrieve Index Pattern from Ingest Manager (elastic#63016)
  [Endpoint] Host Details Policy Response Panel (elastic#63518)
  [Uptime] Certificate expiration threshold settings (elastic#63682)
  Refactor saved object types to use `namespaceType` (elastic#63217)
  [SIEM][CASE] Create comments sequentially (elastic#63692)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:fix v7.8.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Maps] term join metrics with same aggregation and field have name collision
4 participants