Use Instant to track session start time #3761
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
findepi
approved these changes
May 16, 2020
presto-main/src/main/java/io/prestosql/testing/TestingConnectorSession.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/testing/TestingConnectorSession.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/testing/TestingConnectorSession.java
Outdated
Show resolved
Hide resolved
Member
There was a problem hiding this comment.
In session builder you can leave an overload taking millis, this would improve readability of the code here.
Member
Author
There was a problem hiding this comment.
We want to discourage usages where it's constructing the Instant from millis. Ideally, all these calls should use Java's datetime to instant conversions instead of using Joda's DateTime .
Member
There was a problem hiding this comment.
add @Deprecated too?
(should we have a static check?)
Member
Author
There was a problem hiding this comment.
What do you mean by a static check?
Member
There was a problem hiding this comment.
Using deprecated in the Javadoc without the annotation.
Member
Author
There was a problem hiding this comment.
That makes sense. Is it even feasible to do that with checkstyle?
This is needed for higher precision version of the special datetime functions (localtimestamp, current_timestamp, etc), since Instant can provide nanosecond precision vs the current value in milliseconds.
martint
force-pushed
the
session-start-time
branch
from
936634c to
18ff2e5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is needed for higher precision version of the special
datetime functions (localtimestamp, current_timestamp, etc),
since Instant can provide nanosecond precision vs the current
value in milliseconds.