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

Update documentation to mention PARAMETRIC_DATETIME client capabilities header #5068

Open
alec-heif opened this issue Sep 3, 2020 · 7 comments
Labels

Comments

@alec-heif
Copy link
Contributor

The following query

SELECT cast(TIMESTAMP '2020-09-01 12:00:00.123456789' as TIMESTAMP(9))

gives the following result when run against the <coordinator_host>/v1/statement endpoint:

"2020-09-01 12:00:00.123"

on prestosql 340

This is in contrast to the documentation here which states that the result should be

"2020-09-01 12:00:00.123456789"

FWIW the presto CLI handles this query correctly:

presto> SELECT cast(TIMESTAMP '2020-09-01 12:00:00.123456789' as TIMESTAMP(9));
             _col0
-------------------------------
 2020-09-01 12:00:00.123456789

and even handles the non-parameterized version correctly as well:

presto> SELECT TIMESTAMP '2020-09-01 12:00:00.123456789';
             _col0
-------------------------------
 2020-09-01 12:00:00.123456789
(1 row)
@alec-heif alec-heif added bug Something isn't working correctness labels Sep 3, 2020
@martint
Copy link
Member

martint commented Sep 3, 2020

When we added this feature, we had to preserve backward compatibility for old clients that did not support arbitrary precision. In order to get the new behaviors, client have to declare they support the PARAMETRIC_DATETIME capability via the X-Presto-Client-Capabilities header.

@alec-heif
Copy link
Contributor Author

ah I see, thanks for the clarification.

do you think it makes sense to add this advice to either https://prestosql.io/docs/current/functions/datetime.html or https://prestosql.io/docs/current/language/timestamp.html?highlight=timestamp? or will this be a non-issue in 341 now that the PR breaking backcompat with legacy semantics is landed?

@alec-heif
Copy link
Contributor Author

to close the loop I can confirm that the full results are returned when -H "X-Presto-Client-Capabilities: PARAMETRIC_DATETIME" is included so going to retitle this issue as a documentation update :)

@alec-heif alec-heif changed the title Presto REST API drops precision for variable length timestamps Update documentation to mention PARAMETRIC_DATETIME client capabilities header Sep 3, 2020
@alec-heif alec-heif added docs and removed bug Something isn't working correctness labels Sep 3, 2020
@martint
Copy link
Member

martint commented Sep 3, 2020

do you think it makes sense to add this advice to either https://prestosql.io/docs/current/functions/datetime.html or https://prestosql.io/docs/current/language/timestamp.html?highlight=timestamp

No, that should be mentioned in the Clients or Developer section. It's something users shouldn't be concerned with, as it's an implementation detail of the client itself. We don't have any place for that information right now, so we'll have to create something. cc @mosabua

@findepi
Copy link
Member

findepi commented Sep 4, 2020

@alec-heif curious, do you use a client library or interact with REST API directly?

@alec-heif
Copy link
Contributor Author

@findepi I use the pyhive client library

@findepi
Copy link
Member

findepi commented Sep 4, 2020

@alec-heif i created dropbox/PyHive#363 as a follow up to this discussion.
thanks for reminding about this.

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

No branches or pull requests

3 participants