feat(explore): Allow using time formatter on temporal columns in data table #18569

kgabryje · 2022-02-03T13:41:54Z

SUMMARY

Enable switching between displaying epoch (original value) and time formatted value (format: YYYY-mm-dd HH:MM:SS).

Temporal columns in data table (tabs "Data" and "Samples") now have a clickable "gear" icon, which opens a popover that allows switching between 2 display modes.

Selected display mode is specific to each column - user can select different display modes for each temporal column. The selection is also specific to datasource, meaning if there are temporal columns that have the same name in 2 datasources, those columns can have different display modes.

Selection is saved in local storage - when user reloads the page, changes hart config etc., it will be persisted.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2022-02-03.at.14.27.52.mov

TESTING INSTRUCTIONS

Create a chart with some temporal columns
Open data table below the chart - temporal columns should have "gear" icons on their left sides
Click the icon and select "Formatted time"
Verify that the displayed values for that column have changed from long numbers to human readable dates
Reload the page, verify that your selections have been saved
Switch between datasources, verify that the selections are specific for each datasource
Perform the above testing instructions for charts using V1 API and legacy API

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

https://app.shortcut.com/preset/story/33355/explore-south-table-timestamp-column-in-data-panel-did-not-show-correctly

CC @rusackas @jinghua-qa @kasiazjc

… table

codecov · 2022-02-03T13:55:59Z

Codecov Report

Merging #18569 (e961149) into master (a80efa6) will decrease coverage by 0.08%.
The diff coverage is 67.48%.

❗ Current head e961149 differs from pull request most recent head 618a3ef. Consider uploading reports for the commit 618a3ef to get more accurate results

@@            Coverage Diff             @@
##           master   #18569      +/-   ##
==========================================
- Coverage   66.29%   66.21%   -0.09%     
==========================================
  Files        1594     1596       +2     
  Lines       62623    62656      +33     
  Branches     6312     6314       +2     
==========================================
- Hits        41518    41488      -30     
- Misses      19456    19520      +64     
+ Partials     1649     1648       -1

Flag	Coverage Δ
javascript	`51.31% <67.74%> (-0.03%)`	⬇️
presto	`?`

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

Impacted Files	Coverage Δ
...frontend/src/SqlLab/components/ResultSet/index.tsx	`50.73% <ø> (ø)`
...rset-frontend/src/components/ListView/ListView.tsx	`96.25% <ø> (ø)`
...ols/DndColumnSelectControl/ColumnSelectPopover.tsx	`3.94% <0.00%> (+0.05%)`	⬆️
...ColumnSelectControl/ColumnSelectPopoverTrigger.tsx	`65.51% <ø> (+1.00%)`	⬆️
.../controls/DndColumnSelectControl/OptionWrapper.tsx	`63.41% <ø> (ø)`
...et-frontend/src/explore/reducers/exploreReducer.js	`25.71% <0.00%> (-7.62%)`	⬇️
...t-frontend/src/explore/reducers/getInitialState.ts	`0.00% <ø> (ø)`
superset-frontend/src/utils/localStorageHelpers.ts	`90.00% <ø> (ø)`
superset/charts/api.py	`85.93% <ø> (ø)`
superset/charts/schemas.py	`99.34% <ø> (ø)`
... and 26 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 a80efa6...618a3ef. Read the comment docs.

kgabryje · 2022-02-03T17:17:22Z

/testenv up

github-actions · 2022-02-03T17:19:31Z

@kgabryje Ephemeral environment spinning up at http://54.71.136.162:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

villebro

First pass comments. Works really nicely! A minor wording change proposal + I recommend also adding it to legacy charts due to the minor needed changes.

superset-frontend/src/explore/components/DataTableControl/index.tsx

superset-frontend/src/explore/components/DataTablesPane/index.tsx

superset/views/core.py

superset/viz.py

kgabryje · 2022-02-04T15:52:52Z

/testenv up

github-actions · 2022-02-04T15:57:32Z

@kgabryje Ephemeral environment spinning up at http://54.203.1.9:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

villebro

Two last comments, other than that LGTM!

superset-frontend/src/explore/components/DataTableControl/index.tsx

superset/views/core.py

zhaoyongjie · 2022-02-07T14:32:32Z

Thanks for your hard work. Before I reviewed the code, I did some experiments.

Let us use this SQL(PG) to make a virtual dataset, then open it in explore.

select
  '2022-01-03 00:00:00'::TIMESTAMP as datetime0,
  '2022-02-07 00:00:00'::TIMESTAMP as datetime1
UNION ALL
  SELECT '2022-01-04 00:00:00'::TIMESTAMP,
  '2022-02-08 00:00:00'::TIMESTAMP
UNION ALL
  SELECT '2022-01-05 00:00:00'::TIMESTAMP,
  '2022-02-09 00:00:00'::TIMESTAMP
UNION ALL
  SELECT '2022-01-06 00:00:00'::TIMESTAMP,
  '2022-02-10 00:00:00'::TIMESTAMP
UNION ALL
  SELECT '2022-01-07 00:00:00'::TIMESTAMP,
  '2022-02-11 00:00:00'::TIMESTAMP
UNION ALL
  SELECT '2022-01-08 00:00:00'::TIMESTAMP,
  '2022-02-12 00:00:00'::TIMESTAMP
UNION ALL
  SELECT '2022-01-09 00:00:00'::TIMESTAMP,
  '2022-02-13 00:00:00'::TIMESTAMP
UNION ALL
  SELECT '2022-01-10 00:00:00'::TIMESTAMP,
  '2022-02-14 00:00:00'::TIMESTAMP
UNION ALL
  SELECT '2022-01-11 00:00:00'::TIMESTAMP,
  '2022-02-15 00:00:00'::TIMESTAMP
UNION ALL
  SELECT '2022-01-12 00:00:00'::TIMESTAMP,
  '2022-02-16 00:00:00'::TIMESTAMP
UNION ALL
  SELECT '2022-01-13 00:00:00'::TIMESTAMP,
  '2022-02-17 00:00:00'::TIMESTAMP
UNION ALL
  SELECT '2022-01-14 00:00:00'::TIMESTAMP,
  '2022-02-18 00:00:00'::TIMESTAMP
UNION ALL
  SELECT '2022-01-15 00:00:00'::TIMESTAMP,
  '2022-02-19 00:00:00'::TIMESTAMP
UNION ALL
  SELECT '2022-01-16 00:00:00'::TIMESTAMP,
  '2022-02-20 00:00:00'::TIMESTAMP

Select table, put datatime0 and datetime1 to groupby control, put count(*) to metric, select month grain
viz result and query result is different.

This problem was not actually introduced by this PR, but it is a long-standing one.

let us check SQL, we can see time function only apply on main time column, while datetime1 wrongly uses the time drill-down function in the frontend.

IMO, We'd better fix the original issue before we merge this PR into master.

SELECT DATE_TRUNC('month', datetime0) AS datetime0,
       datetime1 AS datetime1,
       count(*) AS count
FROM
  (select '2022-01-03 00:00:00'::TIMESTAMP as datetime0,
          '2022-02-07 00:00:00'::TIMESTAMP as datetime1
   UNION ALL SELECT '2022-01-04 00:00:00'::TIMESTAMP,
                    '2022-02-08 00:00:00'::TIMESTAMP
   UNION ALL SELECT '2022-01-05 00:00:00'::TIMESTAMP,
                    '2022-02-09 00:00:00'::TIMESTAMP
   UNION ALL SELECT '2022-01-06 00:00:00'::TIMESTAMP,
                    '2022-02-10 00:00:00'::TIMESTAMP
   UNION ALL SELECT '2022-01-07 00:00:00'::TIMESTAMP,
                    '2022-02-11 00:00:00'::TIMESTAMP
   UNION ALL SELECT '2022-01-08 00:00:00'::TIMESTAMP,
                    '2022-02-12 00:00:00'::TIMESTAMP
   UNION ALL SELECT '2022-01-09 00:00:00'::TIMESTAMP,
                    '2022-02-13 00:00:00'::TIMESTAMP
   UNION ALL SELECT '2022-01-10 00:00:00'::TIMESTAMP,
                    '2022-02-14 00:00:00'::TIMESTAMP
   UNION ALL SELECT '2022-01-11 00:00:00'::TIMESTAMP,
                    '2022-02-15 00:00:00'::TIMESTAMP
   UNION ALL SELECT '2022-01-12 00:00:00'::TIMESTAMP,
                    '2022-02-16 00:00:00'::TIMESTAMP
   UNION ALL SELECT '2022-01-13 00:00:00'::TIMESTAMP,
                    '2022-02-17 00:00:00'::TIMESTAMP
   UNION ALL SELECT '2022-01-14 00:00:00'::TIMESTAMP,
                    '2022-02-18 00:00:00'::TIMESTAMP
   UNION ALL SELECT '2022-01-15 00:00:00'::TIMESTAMP,
                    '2022-02-19 00:00:00'::TIMESTAMP
   UNION ALL SELECT '2022-01-16 00:00:00'::TIMESTAMP,
                    '2022-02-20 00:00:00'::TIMESTAMP) AS virtual_table
GROUP BY DATE_TRUNC('month', datetime0),
         datetime1
ORDER BY count DESC
LIMIT 1000;

villebro · 2022-02-07T14:37:05Z

@zhaoyongjie the reason for this is that existing users are currently relying on the timegrain only applying to the primary temporal column. See the discussion here: #17239 We're actually currently exploring the possibility of introducing a time grain option in the column control, after which we will no longer be restricted to only having a single time grained temporal column.

zhaoyongjie · 2022-02-07T14:51:17Z

@zhaoyongjie the reason for this is that existing users are currently relying on the timegrain only applying to the primary temporal column. See the discussion here: #17239 We're actually currently exploring the possibility of introducing a time grain option in the column control, after which we will no longer be restricted to only having a single time grained temporal column.

@villebro Thanks for the mention! I applied time format in the customize panel to resolve the inconsistency.

superset-frontend/src/explore/reducers/exploreReducer.js

villebro

LGTM!

github-actions · 2022-02-09T09:30:31Z

Ephemeral environment shutdown and build artifacts deleted.

kgabryje added 3 commits February 3, 2022 11:20

feat(explore): Allow using time formatter on temporal columns in data…

e7a4dd2

… table

Fix data table loading

f197780

Return colnames and coltypes from results request

74976db

kgabryje requested review from zhaoyongjie, etr2460, pkdotson, villebro and geido February 3, 2022 13:41

pull-request-size bot added the size/L label Feb 3, 2022

kgabryje added 2 commits February 3, 2022 15:03

Fix types

f692a18

Fix tests

410adc3

villebro reviewed Feb 4, 2022

View reviewed changes

Fix copy button

9a3a26a

pull-request-size bot added size/XL and removed size/L labels Feb 4, 2022

Fix df is none

ef2f7f5

villebro reviewed Feb 4, 2022

View reviewed changes

superset-frontend/src/explore/components/DataTableControl/index.tsx Outdated Show resolved Hide resolved

superset/views/core.py Outdated Show resolved Hide resolved

kgabryje added 2 commits February 7, 2022 14:17

Fix test

1503925

Address comments

18ebae0

zhaoyongjie reviewed Feb 7, 2022

View reviewed changes

superset-frontend/src/explore/reducers/exploreReducer.js Outdated Show resolved Hide resolved

zhaoyongjie reviewed Feb 7, 2022

View reviewed changes

superset-frontend/src/explore/reducers/exploreReducer.js Outdated Show resolved Hide resolved

zhaoyongjie approved these changes Feb 7, 2022

View reviewed changes

Move useTimeFormattedColumns out of useTableColumns

9a77d01

kgabryje force-pushed the feat/data-table-time-format branch from 06b9494 to 9a77d01 Compare February 8, 2022 12:21

villebro approved these changes Feb 8, 2022

View reviewed changes

Make reducer more readable

618a3ef

kgabryje merged commit 830f2e7 into apache:master Feb 9, 2022

kgabryje added the lts-v1 label Mar 23, 2022

This was referenced Jul 12, 2022

Explore Datatable crash when dimension is empty #20679

Closed

fix(explore): datatable crash when dimension is empty #20680

Merged

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(explore): Allow using time formatter on temporal columns in data table #18569

feat(explore): Allow using time formatter on temporal columns in data table #18569

kgabryje commented Feb 3, 2022

codecov bot commented Feb 3, 2022 •

edited

kgabryje commented Feb 3, 2022

github-actions bot commented Feb 3, 2022

villebro left a comment

kgabryje commented Feb 4, 2022

github-actions bot commented Feb 4, 2022

villebro left a comment

zhaoyongjie commented Feb 7, 2022 •

edited

villebro commented Feb 7, 2022

zhaoyongjie commented Feb 7, 2022 •

edited

villebro left a comment

github-actions bot commented Feb 9, 2022

feat(explore): Allow using time formatter on temporal columns in data table #18569

feat(explore): Allow using time formatter on temporal columns in data table #18569

Conversation

kgabryje commented Feb 3, 2022

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Feb 3, 2022 • edited

Codecov Report

kgabryje commented Feb 3, 2022

github-actions bot commented Feb 3, 2022

villebro left a comment

Choose a reason for hiding this comment

kgabryje commented Feb 4, 2022

github-actions bot commented Feb 4, 2022

villebro left a comment

Choose a reason for hiding this comment

zhaoyongjie commented Feb 7, 2022 • edited

villebro commented Feb 7, 2022

zhaoyongjie commented Feb 7, 2022 • edited

villebro left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 9, 2022

codecov bot commented Feb 3, 2022 •

edited

zhaoyongjie commented Feb 7, 2022 •

edited

zhaoyongjie commented Feb 7, 2022 •

edited