-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix assertions for old Hive versions in TestHiveViews for from_utc_timestamp #8860
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the best we can do. It's better to document such things as tests rather than leave them unknown.
What is the Hive behaviour though - I can't understand how it's arriving at those specific values?
LGTM % nits.
|
||
private boolean isNewFromUtcTimestampSemantics() | ||
{ | ||
// it appears from_utc_timestamp semantics in Hive changes some time on the way. The guess is that it happened |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Should we update the comment to be statement instead of a guess once the tests are green with this assumption.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, maybe reversing the function logic (i.e. return !(condition)
) + rename to isOld
may make more sense given that the old behaviour is the "exceptional" behaviour and new behaviour is the expected one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reveresed the condition. As for guessing, the green pass does not tell us much. I see that behaviour is different for 3.1 and for older ones. But I do not know if the behavior switch was exactly on version 3.1
It looks like on old Hive values are shifted by timezone offset of Hive's VM. It is using Asia/Kathmandu which is +5:30 or 5:45 vs UTC. |
ff44129
to
e411557
Compare
No description provided.