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

datetime-diff function for redshift #26758

Merged
merged 31 commits into from
Dec 8, 2022
Merged

datetime-diff function for redshift #26758

merged 31 commits into from
Dec 8, 2022

Conversation

calherries
Copy link
Contributor

@calherries calherries commented Nov 25, 2022

Adds support for the datetime-diff function for redshift.

Original datetime-diff PR: #25722


This change is Reviewable

@calherries calherries changed the title Add redshift implementation datetime-diff function for redshift Nov 25, 2022
@calherries calherries added the backport Automatically create PR on current release branch on merge label Nov 30, 2022
@codecov
Copy link

codecov bot commented Dec 1, 2022

Codecov Report

Base: 64.35% // Head: 64.34% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (a0ef13a) compared to base (f342fe1).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #26758      +/-   ##
==========================================
- Coverage   64.35%   64.34%   -0.02%     
==========================================
  Files        3215     3217       +2     
  Lines       93099    93153      +54     
  Branches    11823    11851      +28     
==========================================
+ Hits        59917    59939      +22     
- Misses      28456    28479      +23     
- Partials     4726     4735       +9     
Flag Coverage Δ
back-end 85.49% <ø> (ø)

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

Impacted Files Coverage Δ
...ntend/src/metabase/entities/snippet-collections.js 16.66% <0.00%> (-28.79%) ⬇️
...abase/core/components/TextArea/TextArea.styled.tsx 60.00% <0.00%> (-20.00%) ⬇️
...ntend/src/metabase/core/components/Input/Input.tsx 66.66% <0.00%> (-15.16%) ⬇️
frontend/src/metabase/lib/redux.js 79.33% <0.00%> (-7.44%) ⬇️
...metabase/components/ListField/ListField.styled.tsx 55.55% <0.00%> (-6.95%) ⬇️
...rc/metabase/core/components/Input/Input.styled.tsx 86.66% <0.00%> (-5.65%) ⬇️
frontend/src/metabase/lib/entities.js 79.12% <0.00%> (-4.95%) ⬇️
frontend/src/metabase/entities/search.js 77.27% <0.00%> (-4.55%) ⬇️
...leSelectListField/SingleSelectListField.styled.tsx 45.45% <0.00%> (-4.55%) ⬇️
frontend/src/metabase/redux/requests.js 82.14% <0.00%> (-3.58%) ⬇️
... and 78 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@calherries calherries requested a review from a team December 6, 2022 09:40
@calherries calherries marked this pull request as ready for review December 6, 2022 09:40
@calherries calherries mentioned this pull request Dec 6, 2022
13 tasks
Copy link
Contributor

@qnkhuat qnkhuat left a comment

Choose a reason for hiding this comment

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

why does the implementation of postgres does not work for redshift?
and should it be a comment somewhere?

@calherries
Copy link
Contributor Author

@qnkhuat 46e4cd4

@calherries calherries merged commit 407d83c into master Dec 8, 2022
@calherries calherries deleted the datetime-diff-redshift branch December 8, 2022 10:30
github-actions bot pushed a commit that referenced this pull request Dec 8, 2022
* Add redshift implementation

* Add failing tests

* Fix bigquery

* Formatting

* fix? set-param for redshift

* Replace literals for datetime-diff-time-zones-test

* Actually use fields

* Fix typo

* Rename dataset

* Add type/DateTimeWithTZ column, since redshift doesn't like type/DateTimeWithZoneOffset

* Cast to timestamp for week diff

* Revert "fix? set-param for redshift"

This reverts commit 235f3ac.

* Fix leap years

* Remove redundant date-trunc

* Formatting

* Fix mismatched types

* Fix extract

* Add comment for why redshift needs an implementation

Co-authored-by: Ngoc Khuat <qn.khuat@gmail.com>
metabase-bot bot added a commit that referenced this pull request Dec 9, 2022
* Add redshift implementation

* Add failing tests

* Fix bigquery

* Formatting

* fix? set-param for redshift

* Replace literals for datetime-diff-time-zones-test

* Actually use fields

* Fix typo

* Rename dataset

* Add type/DateTimeWithTZ column, since redshift doesn't like type/DateTimeWithZoneOffset

* Cast to timestamp for week diff

* Revert "fix? set-param for redshift"

This reverts commit 235f3ac.

* Fix leap years

* Remove redundant date-trunc

* Formatting

* Fix mismatched types

* Fix extract

* Add comment for why redshift needs an implementation

Co-authored-by: Ngoc Khuat <qn.khuat@gmail.com>

Co-authored-by: Cal Herries <39073188+calherries@users.noreply.github.com>
Co-authored-by: Ngoc Khuat <qn.khuat@gmail.com>
@calherries calherries self-assigned this Dec 9, 2022
@calherries calherries added this to the 0.45.2 milestone Dec 13, 2022
calherries added a commit that referenced this pull request Dec 14, 2022
@flamber flamber removed this from the 0.45.2 milestone Dec 21, 2022
calherries added a commit that referenced this pull request Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants