fix: Pinot query generation with time granularities: WEEKS/MONTHS/QUARTERS/YEARS #12536

xiangfu0 · 2021-01-15T03:59:01Z

SUMMARY

Fixing the bug that time granularity doesn't work for WEEKS/MONTHS/QUARTERS/YEARS.
Pinot internally has two time converter functions: timeConverter and dateTrunc.
Pinot timeConverter has better performance comparing to dateTrunc function when querying for granularity DAYS/MINUTES/SECONDS/... as it uses arithmetic ops for it, however, this function doesn't support conversion to WEEKS/MONTHS/QUARTERS/YEARS.

So we need to use dateTrunc for querying with those granularities.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before: error when send queries using granularity: week/month/quarter/year

Query generated:

SELECT DATETIMECONVERT(SecondsSinceEpoch, '1:SECONDS:EPOCH', '1:SECONDS:EPOCH', '1:WEEKS'),
       count(DepDel15) AS count_1
FROM "airlineStats"
GROUP BY DATETIMECONVERT(SecondsSinceEpoch, '1:SECONDS:EPOCH', '1:SECONDS:EPOCH', '1:WEEKS')
LIMIT 10000;

After, query with month:

The query generated is:

SELECT DATETRUNC('month', DaysSinceEpoch * 86400, 'SECONDS'),
       count(TotalAddGTime) AS count_1
FROM "airlineStats"."airlineStats"
GROUP BY DATETRUNC('month', DaysSinceEpoch * 86400, 'SECONDS')
LIMIT 10000;

TEST PLAN

Unit test and integration tests with Pinot quickstart

ADDITIONAL INFORMATION

codecov-io · 2021-01-15T04:59:50Z

Codecov Report

Merging #12536 (9044c1e) into master (3f3a2ad) will decrease coverage by 3.32%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #12536      +/-   ##
==========================================
- Coverage   66.72%   63.40%   -3.33%     
==========================================
  Files        1014      485     -529     
  Lines       49612    29949   -19663     
  Branches     4839        0    -4839     
==========================================
- Hits        33106    18988   -14118     
+ Misses      16383    10961    -5422     
+ Partials      123        0     -123

Flag	Coverage Δ
cypress	`?`
javascript	`?`
python	`63.40% <100.00%> (-0.53%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset/db_engine_specs/pinot.py	`70.73% <100.00%> (+2.31%)`	⬆️
superset/db_engines/hive.py	`0.00% <0.00%> (-85.72%)`	⬇️
superset/db_engine_specs/hive.py	`54.61% <0.00%> (-29.24%)`	⬇️
superset/db_engine_specs/presto.py	`70.56% <0.00%> (-11.48%)`	⬇️
superset/views/database/mixins.py	`80.70% <0.00%> (-1.76%)`	⬇️
superset/models/core.py	`88.04% <0.00%> (-0.82%)`	⬇️
...c/dashboard/components/dnd/AddSliceDragPreview.jsx
...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx
superset-frontend/src/constants.ts
...perset-frontend/src/components/AlteredSliceTag.jsx
... and 525 more

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 3f3a2ad...9044c1e. Read the comment docs.

junlincc · 2021-01-15T05:00:12Z

thanks for the PR and fixing the test! @fx19880617

cc: @betodealmeida

villebro

Nice, thanks for the fix @fx19880617 !

xiangfu0 · 2021-01-15T09:06:45Z

Nice, thanks for the fix @fx19880617 !

Many thanks for reviewing and merging the PR!

villebro · 2021-01-15T09:42:25Z

Many thanks for reviewing and merging the PR!

@fx19880617 this just made it in time for the v1.0.0 cut 🙂

…ARS (#12536)

* release: bump to 1.0.0 and CHANGELOG * fix(explore): long metric name display (#12387) * fix(explore): long metric name display * add tooltip to control * chore: Show datasets when search input is empty (#12391) * chore: Fix typo “Rest” to “Reset” (#12392) * chore: upgrade eslint, babel, and prettier (#12393) * feat(explore): add tooltip to timepicker label (#12401) * chore: change Datasource to Dataset in Explore ui (#12402) * chore(explore):change dataset to datasource in ui * modal * Add space * Changing it back🤦🏾‍♀️ * Chargeback * fix: Refresh Interval Modal dropdown (#12406) * fix(native-filters): incorrect queriesData state (#12409) * refactor: from superset.utils.core break down date_parser (#12408) * Fixes control panel fields styling (#12236) (#12326) * feat: Resizable dataset and controls panels on Explore view (#12411) * Implement resizable panels on explore view * Optimize chart rendering while resizing * Make dataset column narrower Co-authored-by: Evan Rusackas <evan@preset.io> * fix(dashboard): artefacts shown while drag and dropping deck.gl charts (#12418) * [12181] Fix artifacts while drag and dropping deck.gl charts. * Run prettier * bump superset-ui packages for rolling window change (#12426) * chore: bump superset-ui deckgl plugin (#12466) * fix: do not show vertical scrollbar for charts in dashboard (#12478) * fix: do not show vertical scrollbar for charts in dashboard * Proper fix for #11419 Co-authored-by: Jesse Yang <jesse.yang@airbnb.com> * fix(dashboard): use datasource id from slice metadata (#12483) * fix(timepicker): make pyparsing thread safe (#12489) * fix: make pyparsing thread safe * remove parenthesis for decorator * fix (SQL Lab): disappearing results on tab switch (#12472) * fix (SQL Lab): disappearing results on tab switch * Remove state * Fix test * fix: import ZIP files that have been modified (#12425) * fix: import ZIP files that have been modified * Add unit test * update changelog with rc2 entries * fix: impose dataset ownership check on old API (#12491) * fix: impose dataset ownership check on old API * update UPDATING.md * partially protect the old MVC also * prevent metric and column add and update * ci: remove refs/tags from docker tags on a release (#12518) * ci: remove refs/tags from docker tags on a release * wider head * fix: lowercase all columns in examples (#12530) * fix(explore): time table control panel (#12532) * fix(explore): Add Time section back to FilterBox (#12537) * Fixing Pinot queries for time granularities: WEEKS/MONTHS/QUARTERS/YEARS (#12536) * fix: Select options overflowing Save chart modal on Explore view (#12522) * Fix select options overflowing modal * fix unit test Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com> * Fix list filters vertical alignment (#12497) * feat(db-engine): Add support for Apache Solr (#12403) * [db engine] Add support for Apache Solr * Fixing typo * chore: rename docker image in build_docker_image.sh, docker-compose.yml and helm values.yaml (#12337) * add rc3 changelog entries * fix: Popover closes on change of dropdowns values (#12410) * fix: Add MAX_SQL_ROW value to LIMIT_DROPDOWN (#12555) * fix(viz): missing groupby and broken adhoc metrics for boxplot (#12556) * fix: height on grid results (#12558) * fix: case expression should not have double quotes (#12562) * Fix 500 error when loading dashboards with slice having deleted dataset (#12535) * add rc4 changelog entries * Fixed typo on line 348 * Added files Co-authored-by: Daniel Gaspar <danielvazgaspar@gmail.com> Co-authored-by: Yongjie Zhao <yongjie.zhao@gmail.com> Co-authored-by: Geido <60598000+geido@users.noreply.github.com> Co-authored-by: Junlin Chen <junlin@preset.io> Co-authored-by: Jesse Yang <jesse.yang@airbnb.com> Co-authored-by: Agata Stawarz <47450693+agatapst@users.noreply.github.com> Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com> Co-authored-by: Evan Rusackas <evan@preset.io> Co-authored-by: Kasia Kucharczyk <2536609+kkucharc@users.noreply.github.com> Co-authored-by: Phillip Kelley-Dotson <pkelleydotson@yahoo.com> Co-authored-by: Grace Guo <grace.guo@airbnb.com> Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com> Co-authored-by: Xiang Fu <fx19880617@gmail.com> Co-authored-by: Ahmed Adel <github@aadel.io> Co-authored-by: Amit Miran <47772523+amitmiran137@users.noreply.github.com> Co-authored-by: Hugh A. Miles II <hughmil3s@gmail.com> Co-authored-by: Shuyao Bi <shuyaob@andrew.cmu.edu> Co-authored-by: Lyndsi Kay Williams <lyndsikaywilliams@Lyndsis-MacBook-Pro.local>

pull-request-size bot added the size/M label Jan 15, 2021

junlincc added the hold:testing! On hold for testing label Jan 15, 2021

xiangfu0 force-pushed the fixing_date_trunc_in_pinot branch from 41ce449 to 714e0d7 Compare January 15, 2021 04:56

junlincc removed the hold:testing! On hold for testing label Jan 15, 2021

Fixing Pinot queries for time granularities: WEEKS/MONTHS/QUARTERS/YEARS

9044c1e

xiangfu0 force-pushed the fixing_date_trunc_in_pinot branch from 714e0d7 to 9044c1e Compare January 15, 2021 07:12

villebro approved these changes Jan 15, 2021

View reviewed changes

villebro merged commit 8f940ae into apache:master Jan 15, 2021

villebro added the v1.0 label Jan 15, 2021

xiangfu0 deleted the fixing_date_trunc_in_pinot branch January 15, 2021 09:06

villebro pushed a commit that referenced this pull request Jan 15, 2021

Fixing Pinot queries for time granularities: WEEKS/MONTHS/QUARTERS/YE…

5f2de1d

…ARS (#12536)

mistercrunch added 🍒 1.0.0 Cherry-picked to 1.0.0 🍒 1.0.1 Cherry-picked to 1.0.1 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 First shipped in 1.2.0 labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Pinot query generation with time granularities: WEEKS/MONTHS/QUARTERS/YEARS #12536

fix: Pinot query generation with time granularities: WEEKS/MONTHS/QUARTERS/YEARS #12536

Uh oh!

xiangfu0 commented Jan 15, 2021 •

edited

Loading

Uh oh!

codecov-io commented Jan 15, 2021 •

edited

Loading

Uh oh!

junlincc commented Jan 15, 2021 •

edited

Loading

Uh oh!

villebro left a comment

Uh oh!

xiangfu0 commented Jan 15, 2021

Uh oh!

villebro commented Jan 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fix: Pinot query generation with time granularities: WEEKS/MONTHS/QUARTERS/YEARS #12536

fix: Pinot query generation with time granularities: WEEKS/MONTHS/QUARTERS/YEARS #12536

Uh oh!

Conversation

xiangfu0 commented Jan 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

Uh oh!

codecov-io commented Jan 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

junlincc commented Jan 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

villebro left a comment

Choose a reason for hiding this comment

Uh oh!

xiangfu0 commented Jan 15, 2021

Uh oh!

villebro commented Jan 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

xiangfu0 commented Jan 15, 2021 •

edited

Loading

codecov-io commented Jan 15, 2021 •

edited

Loading

junlincc commented Jan 15, 2021 •

edited

Loading