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

fix: Query execution time is displayed as invalid date #19605

Conversation

diegomedina248
Copy link
Contributor

SUMMARY

The Time column in the Query History is being displayed as an Invalid date.

The reason being the underlying data type is numeric with precision, to get sub-second precision.
That means the returned json value is a string (to keep the precision) and in turn, it fails the conversion to a moment object.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:
Screen Shot 2022-04-07 at 20 23 25

After:
new

TESTING INSTRUCTIONS

  1. Go to SQL lab -> SQL Editor
  2. Run any query
  3. Go to SQL lab -> Query History

Ensure the time column is displaying the proper start time.

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

@yousoph
Copy link
Member

yousoph commented Apr 7, 2022

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2022

@yousoph Container image not yet published for this PR. Please try again when build is complete.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2022

@yousoph Ephemeral environment creation failed. Please check the Actions logs for details.

@yousoph
Copy link
Member

yousoph commented Apr 8, 2022

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2022

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

@codecov
Copy link

codecov bot commented Apr 8, 2022

Codecov Report

Merging #19605 (4eaeea0) into master (e632b82) will decrease coverage by 12.55%.
The diff coverage is 100.00%.

❗ Current head 4eaeea0 differs from pull request most recent head 84cdf5d. Consider uploading reports for the commit 84cdf5d to get more accurate results

@@             Coverage Diff             @@
##           master   #19605       +/-   ##
===========================================
- Coverage   66.47%   53.92%   -12.56%     
===========================================
  Files        1713     1713               
  Lines       64942    64974       +32     
  Branches     6690     6690               
===========================================
- Hits        43171    35034     -8137     
- Misses      20067    28236     +8169     
  Partials     1704     1704               
Flag Coverage Δ
hive 52.91% <100.00%> (+0.04%) ⬆️
mysql ?
postgres ?
presto 52.76% <100.00%> (+0.04%) ⬆️
python 56.77% <100.00%> (-25.60%) ⬇️
sqlite ?
unit 47.99% <96.77%> (+0.04%) ⬆️

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

Impacted Files Coverage Δ
...src/components/FilterableTable/FilterableTable.tsx 71.42% <ø> (ø)
superset/charts/post_processing.py 67.17% <ø> (ø)
superset/utils/pandas_postprocessing/pivot.py 91.17% <ø> (ø)
...e/components/controls/MetricControl/AdhocMetric.js 95.65% <100.00%> (+0.19%) ⬆️
...ols/MetricControl/AdhocMetricEditPopover/index.jsx 76.28% <100.00%> (ø)
superset/queries/api.py 100.00% <100.00%> (ø)
superset/queries/schemas.py 100.00% <100.00%> (ø)
superset/utils/dashboard_import_export.py 0.00% <0.00%> (-100.00%) ⬇️
superset/key_value/commands/upsert.py 0.00% <0.00%> (-89.59%) ⬇️
superset/key_value/commands/update.py 0.00% <0.00%> (-89.37%) ⬇️
... and 274 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 e632b82...84cdf5d. Read the comment docs.

Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

At some point we should probably move away from moment. But that saddens me. An old and loyal friend.

@rusackas rusackas closed this Apr 8, 2022
@rusackas rusackas reopened this Apr 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2022

Ephemeral environment shutdown and build artifacts deleted.

@diegomedina248 diegomedina248 force-pushed the fix/query-execution-time-is-displayed-as-invalid-date branch from 584afe7 to 0e45ea0 Compare April 12, 2022 02:23
@diegomedina248 diegomedina248 force-pushed the fix/query-execution-time-is-displayed-as-invalid-date branch from 0e45ea0 to 6096d14 Compare April 19, 2022 20:42
@diegomedina248 diegomedina248 force-pushed the fix/query-execution-time-is-displayed-as-invalid-date branch from 6096d14 to 711e183 Compare April 26, 2022 17:22
@pull-request-size pull-request-size bot added size/M and removed size/S labels Apr 26, 2022
@diegomedina248 diegomedina248 force-pushed the fix/query-execution-time-is-displayed-as-invalid-date branch from 711e183 to 38808a8 Compare April 26, 2022 17:45
@zhaoyongjie zhaoyongjie self-requested a review April 27, 2022 03:20
Copy link
Member

@zhaoyongjie zhaoyongjie left a comment

Choose a reason for hiding this comment

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

Thanks for the following up! LGTM.

@zhaoyongjie zhaoyongjie merged commit e3dbe8d into apache:master Apr 27, 2022
sadpandajoe pushed a commit to preset-io/superset that referenced this pull request Apr 29, 2022
* fix: Query execution time is displayed as invalid date

* PR comment

* Fix test

* unify response

* lint

(cherry picked from commit e3dbe8d)
@sadpandajoe
Copy link
Member

🏷️ preset:2022.17

hughhhh pushed a commit to hve-labs/superset that referenced this pull request May 11, 2022
* fix: Query execution time is displayed as invalid date

* PR comment

* Fix test

* unify response

* lint
philipher29 pushed a commit to ValtechMobility/superset that referenced this pull request Jun 9, 2022
* fix: Query execution time is displayed as invalid date

* PR comment

* Fix test

* unify response

* lint
@justinpark justinpark mentioned this pull request Aug 30, 2023
9 tasks
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 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 preset:2022.17 Preset-Patch size/M 🚢 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants