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

trino-python-client returns dtype string for datetime/timestamp fields #32

Closed
RJ3 opened this issue Jun 3, 2020 · 7 comments
Closed
Assignees

Comments

@RJ3
Copy link

RJ3 commented Jun 3, 2020

presto-client returns data type of string for a timestamp field.
Setup:

  • PrestoSQL 333
  • PostgreSQL 12.1
  • presto-client 0.300.0

Tested:

  1. Using the PrestoSQL JDBC driver, the field is returned as a Java timestamp.
  2. Using psycopg2 driver with SQLalchemy directly to Postgres, the field is returned as a python datetime object or pandas timestamp object.
  3. Using presto-client 0.300, the field is returned as a string.

Expectation:
The field should be returned as a python datetime object or pandas timestamp object, akin to the PostgreSQL driver.

Note:
The issue has also been reported for pyhive (no longer under current development). I have not tested PrestoDB's version to see if the other team has also worked on the topic.

@findepi
Copy link
Member

findepi commented Jun 5, 2020

The biggest question is how to avoid breaking users.
For now we should implement napping to datetime behing a toggle (init param).
At some point, we probably should consdier switching the default, as we do not want to maintain inferior representation for ever.
cc @electrum

@alexbrisan
Copy link

@findepi I would like to contribute a fix for this. Would you like me to detail a fix first, or just submit a Pull Request?

@findepi
Copy link
Member

findepi commented Jun 7, 2021

@alexbrisan awesome!

it's worthwhile to have a conversation before doing work, if the work would take a lot of time.
Perhaps this won't be the case here, will it?

Things to consider

  • what do other drivers do? eg. postgresql's or oracle's?
  • do we have a toggle to switch between old & new behavior?
  • i am not expert on date/time/timezones handling in Python, but it seems the standard library doesn't cover all the use-cases (hence pytz exists).
    • this is why checking with oracle's driver can be interesting. PostgreSQL doesn't have SQL standard-conforming timestamp with time zone data type and both Trino and Oracle do.

@alexbrisan
Copy link

I will take a look at the oracle one and come up with a plan!

@ebyhr ebyhr changed the title presto-client returns dtype string for datetime/timestamp fields trino-python-client returns dtype string for datetime/timestamp fields Aug 17, 2021
@buremba
Copy link
Member

buremba commented Aug 27, 2021

The issue cases Superset to fail creating datasets that have DATE/TIMESTAMP columns. I can also send a PR if there is any interest to fix it but #81 seems to fix the issue already.

@mdesmet
Copy link
Contributor

mdesmet commented Apr 8, 2022

I think this issue can be closed as we merged #160.

@ebyhr ebyhr closed this as completed Apr 8, 2022
@ebyhr
Copy link
Member

ebyhr commented Apr 8, 2022

@mdesmet Closed. Thanks for your contribution!

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

No branches or pull requests

6 participants