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

[Bugfix] Postaggregations depending on postaggregations #4017

Merged
merged 1 commit into from Dec 7, 2017

Conversation

Mogball
Copy link
Contributor

@Mogball Mogball commented Dec 6, 2017

Druid post-aggregations can depend on other post-aggregations in their fieldNames, but current behaviour will treat them all as aggregations. Also, post-aggregation order is important because a dependent post-aggregation needs to be defined after post-aggregations it depends on.

This fix will cause superset to search for postaggregations and then aggregations.

@Mogball Mogball force-pushed the jeff/druid_tuple_sketch branch 2 times, most recently from 170adc0 to 8753672 Compare December 6, 2017 02:13
@fabianmenges
Copy link
Contributor

Nice, I noticed that a while back.

@ananthanithya
Copy link

Awesome! We need this fix too.

@mistercrunch
Copy link
Member

Looks solid, merging. BTW longer term fix is to go with Druid SQL on this. Composing expressions is going to be so much easier.

@mistercrunch mistercrunch merged commit cb7c5aa into apache:master Dec 7, 2017
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.21.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.21.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants