-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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(sqla): support for date adhoc filter #16634
Conversation
Codecov Report
@@ Coverage Diff @@
## master #16634 +/- ##
==========================================
- Coverage 76.77% 76.51% -0.27%
==========================================
Files 1003 1003
Lines 53959 53959
Branches 7330 7330
==========================================
- Hits 41427 41286 -141
- Misses 12293 12434 +141
Partials 239 239
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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
(cherry picked from commit 6465ee7)
SUMMARY
When sending a temporal Native Value Filter to a chart with a
DATE
column, the query errors out. This is due all temporal filters being encoded asdatetime.datetime
objects, irrespective of the target column beingDATE
,DATETIME
orTIMESTAMP
(we handle formatting of the temporal value inBaseEngineSpec.convert_dttm
). This error is due to the SqlAlchemyDate
andDatetime
column types not being able to handledatetime
anddate
objects respectively.This PR replaces the SqlAlchemy
Date
type withDateTime
, making it compatible with the logic insqla/models.py
.AFTER
BEFORE
TESTING INSTRUCTIONS
DATE
typeDATE
columnADDITIONAL INFORMATION