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

add presto dttm format in dttm_converter #631

Closed
tarekrached opened this issue Jun 16, 2016 · 2 comments
Closed

add presto dttm format in dttm_converter #631

tarekrached opened this issue Jun 16, 2016 · 2 comments

Comments

@tarekrached
Copy link

Presto doesn't understand ISO 8601 timestamp strings as time-like, so currently, timestamp comparisons in Presto are being executed as lexical varchar comparisons.

This means that TIMESTAMP columns in Presto can't be used as dttm columns. If you do try to use a TIMESTAMP column in Presto as a dttm column, you get a GREATER_THAN_OR_EQUAL(timestamp, varchar) not registered

One solution would be to add a Presto-specific dttm_converter, such as

"CAST('{}' AS TIMESTAMP)".format(dttm.strftime('%Y-%m-%d %H:%M:%S.%f')[:-3]))

This approach was initially proposed in #522, but I'm creating this issue so that we can discuss more broadly.
#506 (or a per-column version) is necessary but not sufficient - if & when it it gets merged, the timestamp comparison with still be performed as a lexical string comparisons in Presto, and that is not going to work for date formats such as MM/DD/YYYY. (e.g. 01/01/2001 < 02/01/2000).

Whatever solution we end up with should not hinder Presto's optimizer from partition pruning and index utilization.

@mistercrunch
Copy link
Member

We now support per-column spec for how to handle date time, it will be in the next release.

@tarekrached
Copy link
Author

Thanks again for your work on this, @mistercrunch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants