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

Transformations: merge will properly handle empty frames and frames with multiple rows where values are overlapping. #27362

Merged
merged 6 commits into from
Sep 8, 2020

Conversation

mckn
Copy link
Contributor

@mckn mckn commented Sep 3, 2020

What this PR does / why we need it:
This PR resolves the issue reported in the following issues: #26908, #26890.


Prior to this fix, incoming data with the following format:

Query A:

Country AgeGroup Sum
United States 50 or over 998
United States 35 - 49 1193
Mexico 0 - 17 1675
Germany 35 - 49 146
Canada 35 - 49 166
Canada 25 - 34 219

Query B:

AgeGroup Count
0 - 17 1
18 - 24 3
25 - 34 2
35 - 49 4
50 or over 2

Did not properly merged right hand side values in to all matching row and the result was the following:

Country AgeGroup Sum Count
United States 50 or over 998 2
United States 35 - 49 1193 4
United States 25 - 34 1580 2
Mexico 0 - 17 1675 1
Germany 35 - 49 146
Canada 35 - 49 166
Canada 25 - 34 219
18 - 24 3

After applying this fix, the merge will return the following:

Country AgeGroup Sum Count
United States 50 or over 998 2
United States 35 - 49 1193 4
United States 25 - 34 1580 2
Mexico 0 - 17 1675 1
Germany 35 - 49 146 4
Canada 35 - 49 166 4
Canada 25 - 34 219 2
18 - 24 3

It will also handle the scenario where all incoming data frames are empty. In this scenario they will be merged into one single empty frame.

If only a couple of the incoming data frames are empty they will be removed prior to doing the merge. So all the frames containing data will be merged into one single frame.


Which issue(s) this PR fixes:
Fixes #26908
Fixes #26890

@mckn mckn added this to the 7.2 milestone Sep 3, 2020
@mckn mckn requested review from torkelo and a team September 3, 2020 15:05
@mckn mckn self-assigned this Sep 3, 2020
@mckn mckn requested review from peterholmberg and removed request for a team September 3, 2020 15:05
@mckn mckn changed the title Transformations: merge will properly handle merge of multiple rows with overlapping values. Transformations: merge will properly handle multiple rows with overlapping values and empty frames. Sep 4, 2020
@mckn mckn changed the title Transformations: merge will properly handle multiple rows with overlapping values and empty frames. Transformations: merge will properly handle empty frames and frames with multiple rows where values are overlapping. Sep 4, 2020
Copy link
Contributor

@peterholmberg peterholmberg left a comment

Choose a reason for hiding this comment

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

Nice work, justa small nit 🙂

@mckn mckn merged commit f22f0a8 into master Sep 8, 2020
@mckn mckn deleted the mckn/merge-fixes branch September 8, 2020 09:52
ryantxu pushed a commit that referenced this pull request Nov 18, 2020
…ith multiple rows where values are overlapping. (#27362)

* wip.

* Fixed issue with merge not behaving exactly as the old table panel did.

* Fixed so empty data frames will be exluded prior to trying to merge the result.

* Changed so if passing only empty frames first will only be returned.

* de-duplication of configuration in tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants