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

Add more time grains #5083

Merged
merged 3 commits into from
May 29, 2018
Merged

Add more time grains #5083

merged 3 commits into from
May 29, 2018

Conversation

betodealmeida
Copy link
Member

This PR adds the "year" time grain to SQLite, and all the time grains supported by Druid SQL.

@codecov-io
Copy link

codecov-io commented May 25, 2018

Codecov Report

Merging #5083 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5083      +/-   ##
==========================================
+ Coverage   77.53%   77.53%   +<.01%     
==========================================
  Files          44       44              
  Lines        8710     8711       +1     
==========================================
+ Hits         6753     6754       +1     
  Misses       1957     1957
Impacted Files Coverage Δ
superset/db_engine_specs.py 53.82% <100%> (+0.07%) ⬆️

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 c18ef89...a7bcb7b. Read the comment docs.

@john-bodley
Copy link
Member

@betodealmeida it seems like the Druid SQL spec is constantly evolving and it seems like TIME_FLOOR was introduced in 0.11.0 (it's absent in 0.10.1 where these use FLOOR(__time TO <granularity>)). Is there a way to make time grains work for the various versions? Note I'm not sure what Druid versions Superset supports.

@@ -1357,6 +1360,27 @@ class DruidEngineSpec(BaseEngineSpec):
engine = 'druid'
inner_joins = False

time_grains = (
Grain('Time Column', _('Time Column'), '{col}', None),
Grain(
Copy link
Member

Choose a reason for hiding this comment

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

nit: can we make this a single line like the others?

Copy link
Member Author

Choose a reason for hiding this comment

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

The linter would then complain. :-/

But I'm changing the grains to use the FLOOR function, and then this unit will go away.

@betodealmeida
Copy link
Member Author

@john-bodley, good catch — I'll update the PR to use FLOOR, since it works in both versions.

@mistercrunch mistercrunch merged commit 6c3e469 into apache:master May 29, 2018
@mistercrunch mistercrunch deleted the more_time_grains branch May 29, 2018 19:43
timifasubaa pushed a commit to timifasubaa/incubator-superset that referenced this pull request May 31, 2018
* Add more time grains

* Use FLOOR

* Fix quotes for lint
timifasubaa pushed a commit to airbnb/superset-fork that referenced this pull request Jul 25, 2018
* Add more time grains

* Use FLOOR

* Fix quotes for lint
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
* Add more time grains

* Use FLOOR

* Fix quotes for lint
@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.

5 participants