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

[Explore][AdhocMetrics] treating floats like doubles for druid versions lower than 11.0.0 #5030

Merged

Conversation

gabe-lyons
Copy link
Contributor

In the AdhocMetric feature, we construct Druid queries by taking a column object and an aggregate from the user and construct a druid JSON blob in the flask app. However, the query object is not always a function of just the column's type and the aggregate chosen.

Depending on the Druid version being used, we want to sum floats using doubleSum in versions
earlier than 11.0.0 and sum floats using floatSum in versions including and after 11.0.0

A great explanation of why this is the case is provided here: https://groups.google.com/forum/#!topic/druid-development/VEYkW_5dWd8

reviewers:
@john-bodley @michellethomas @mistercrunch @betodealmeida

@gabe-lyons gabe-lyons force-pushed the gabe_and_john_add_fix_for_druid_version branch 5 times, most recently from 3227461 to e309218 Compare May 21, 2018 15:52
@gabe-lyons gabe-lyons force-pushed the gabe_and_john_add_fix_for_druid_version branch from e309218 to be99931 Compare May 21, 2018 16:13
@codecov-io
Copy link

codecov-io commented May 21, 2018

Codecov Report

Merging #5030 into master will increase coverage by <.01%.
The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5030      +/-   ##
==========================================
+ Coverage   77.34%   77.35%   +<.01%     
==========================================
  Files          44       44              
  Lines        8671     8677       +6     
==========================================
+ Hits         6707     6712       +5     
- Misses       1964     1965       +1
Impacted Files Coverage Δ
superset/connectors/druid/models.py 80.53% <93.33%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f66dae...be99931. Read the comment docs.

@mistercrunch
Copy link
Member

LGTM

@mistercrunch mistercrunch merged commit 1c9474b into apache:master May 21, 2018
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 21, 2018
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
timifasubaa pushed a commit to timifasubaa/incubator-superset that referenced this pull request May 31, 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.26.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.26.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants