-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Backport Extend param substitution to handle datetimes to 48 (#38695) #39062
Backport Extend param substitution to handle datetimes to 48 (#38695) #39062
Conversation
* Extend param substitution to handle datetimes Prior to this only dates were supported as values for native timestamp field parameters. * Check for exclusion filter when determining alignment unit Co-authored-by: Case Nelson <case@metabase.com>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM do we need to update docs/developer-guide/driver-changelog.md
for align-temporal-unit-with-param-type-and-value
?
@snoe I wondered about that. I'm not sure what to do when backporting driver changes, because we only have a section for the 48.0 release. This is just a warning that we're deprecating the multimethod so I think it's fine to leave in the 49 release and omit it here? |
Looks like there are more failures |
Looks like the entry needs to be made in the changelog for the tests to pass. I'll add it then |
Manually backports: #38695
Also cherry-picks #37813 which is a necessary prerequisite for the fix.
I manually tested this fixes #38037 using the repro steps here #38037 (comment)