Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: time filter should be [start, end) #19166

Merged
merged 7 commits into from
Mar 17, 2022

Conversation

zhaoyongjie
Copy link
Member

@zhaoyongjie zhaoyongjie commented Mar 16, 2022

SUMMARY

This PR fixes a regression issue from the remove legacy SIP-15 interim logic/flags.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before

image

After

image

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

@codecov
Copy link

codecov bot commented Mar 16, 2022

Codecov Report

Merging #19166 (ad3fc6e) into master (e3e03d2) will increase coverage by 0.00%.
The diff coverage is 40.00%.

❗ Current head ad3fc6e differs from pull request most recent head 1d719be. Consider uploading reports for the commit 1d719be to get more accurate results

@@           Coverage Diff           @@
##           master   #19166   +/-   ##
=======================================
  Coverage   66.55%   66.55%           
=======================================
  Files        1646     1646           
  Lines       63617    63617           
  Branches     6471     6471           
=======================================
+ Hits        42339    42340    +1     
+ Misses      19600    19599    -1     
  Partials     1678     1678           
Flag Coverage Δ
hive 52.53% <100.00%> (ø)
mysql 81.88% <100.00%> (+<0.01%) ⬆️
postgres 81.92% <100.00%> (+<0.01%) ⬆️
presto 52.37% <100.00%> (ø)
python 82.36% <100.00%> (+<0.01%) ⬆️
sqlite 81.67% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
.../components/Header/HeaderActionsDropdown/index.jsx 71.92% <ø> (ø)
...c/views/CRUD/data/database/DatabaseModal/index.tsx 32.90% <ø> (ø)
.../src/dashboard/components/gridComponents/Chart.jsx 57.60% <25.00%> (ø)
superset/connectors/sqla/models.py 90.11% <100.00%> (ø)
superset/common/query_context_processor.py 91.46% <0.00%> (+0.47%) ⬆️

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 e3e03d2...1d719be. Read the comment docs.

@@ -308,7 +308,7 @@ def get_time_filter(
if start_dttm:
l.append(col >= self.table.text(self.dttm_sql_literal(start_dttm)))
if end_dttm:
l.append(col <= self.table.text(self.dttm_sql_literal(end_dttm)))
l.append(col < self.table.text(self.dttm_sql_literal(end_dttm)))
Copy link
Member

Choose a reason for hiding this comment

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

Yikes! Thanks @zhaoyongjie for catching this. This was exactly what SIP-15 was fixing.

@zhaoyongjie zhaoyongjie merged commit e4c9a0d into apache:master Mar 17, 2022
@sadpandajoe
Copy link
Member

🏷️ preset:2022.11

sadpandajoe pushed a commit to preset-io/superset that referenced this pull request Mar 31, 2022
@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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels preset:2022.11 size/M 🚢 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants