Dttm format #506
Dttm format #506
Conversation
|
|
|
|
|
|
|
|
|
|
|
|
Minor style fix
|
|
|
|
This reverts commit 6897c38.
|
|
|
All fixes are done, ready for review. A new input area is present when adding a table. You can use it to provide the date time format to use when generating queries and when rendering the UI. |
|
|
||
|
|
||
| def upgrade(): | ||
| try: |
There was a problem hiding this comment.
failing would result in a broken app, let's fail early in the migration script if anywhere
|
Wait should this be a table attribute or a column attribute? Also, I don't think we need it on the Druid side. If formatting is assumed to be heterogenous across tables it can be just as much across date like columns within a table. |
|
For some reason, Landscape check is not running. This seems to affecting all pull requests right now. |
|
FWIW, we have tables with multiple, differently formatted dttm columns, so I'd love to see this at the column level. Seems like this could sit beside the |
|
I think there's still a glitch where depending on the type in the database, things might need to be casted by a database function or not. Say for Presto, I could have:
Somehow it would be great to be able to make it work in all these situations. 3 is important. |
|
The last one would be really useful for us. Especially if it could be and On Thu, 16 Jun 2016 06:27 Maxime Beauchemin, notifications@github.com
Alan Cruickshank |
|
For the record, we want the casting or transformation to be applied to the constant and not the column. It's possible today to create whatever column expression and have Caravel do the time filtering against that expression. This isn't ideal because the database optimizer gets thrown off and can't do partition pruning or use indexes on the column. The ideal scenario is where the user can tell Caravel knows how to prepare the right side of the equation to use against that time-like column. |
|
@mistercrunch, that sounds solid. I'm a bit confused about your earlier comment regarding dates formatted as MM/DD/YYYY. I can see that you can compare those without raising an error (lexical string to string), but short of explicitly declaring a dttm_format, I don't understand how the comparisons could be valid. |
| self.results = self.datasource.query(**query_obj) | ||
| self.query = self.results.query | ||
| df = self.results.df | ||
| if df is None or df.empty: | ||
| raise Exception("No data, review your incantations!") | ||
| else: | ||
| if 'timestamp' in df.columns: | ||
| df.timestamp = pd.to_datetime(df.timestamp, utc=False) | ||
| df.timestamp = pd.to_datetime( | ||
| df.timestamp, utc=False, format=timestamp_format) |
There was a problem hiding this comment.
I like this here as opposed to relying on pandas's to_datetime magic, but it breaks down if we need to use a database side CAST or TO_DATE of some form
| """Returns a string that the database flavor understands as a date""" | ||
| default = "'{}'".format(dttm.strftime('%Y-%m-%d %H:%M:%S.%f')) | ||
| tf = tf or '%Y-%m-%d %H:%M:%S.%f' | ||
| default = "'{}'".format(dttm.strftime(tf)) | ||
| iso = dttm.isoformat() | ||
| d = { |
There was a problem hiding this comment.
i think this previous approach had shortcomings to start with. It assumed that the casting required could be defined on a per-database basis where it cannot since even within a single table you could represent dates in different fashion (epoch, string, native DATETIME, DATE or TIMESTAMP types...).
|
What about two fields instead of one in Column model:
Does that cover all scenarios? Note that we'll need to maintain the current behavior as much as possible, either by pre-populating these fields based on current database defaults in the db migration script, or by leaving the field empty and applying the current default at runtime where the fields are empty. |
|
Intriguing - I like the way this is going. Wouldn't the Thank you for your time on this, @mistercrunch! |
|
@tarekrached Yeah I think |
|
Thanks! The reply is valuable and we are now working on the problems according to the suggestion from @mistercrunch . We are trying to make changes base on the modification we made before. The |
|
Superseeded by #652 |
Add support for custom datetime format for tables.