-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Remove usages of deprecated parseTimestampInLocalTime #11041
Remove usages of deprecated parseTimestampInLocalTime #11041
Conversation
@@ -179,13 +180,13 @@ public void testAllDataTypes() | |||
.containsOnly( | |||
row("\0", Long.MIN_VALUE, Bytes.fromHexString("0x00").array(), false, 0f, Double.MIN_VALUE, | |||
Float.MIN_VALUE, "[0]", "0.0.0.0", Integer.MIN_VALUE, "[0]", "{\"\\u0000\":-2147483648,\"a\":0}", | |||
"[0]", "\0", parseTimestampInLocalTime("1970-01-01 00:00:00.0", PRODUCT_TESTS_TIME_ZONE), |
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.
so the PRODUCT_TESTS_TIME_ZONE
was completely ignored here?
parseMillis
javadoc:
* Parses a datetime from the given text, returning the number of
* milliseconds since the epoch, 1970-01-01T00:00:00Z.
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.
No, it wasn't. It had to match JVM time zone though (because of how java.sql.Timestamp
internal repr works), and now this is done automatically via Timestamp.valueOf(LocalDateTime)
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.
No, it wasn't. It had to match JVM time zone though (because of how java.sql.Timestamp internal repr works)
Looking at the:
public static Timestamp parseTimestampInLocalTime(String value, DateTimeZone timeZone)
{
return new Timestamp(DATE_TIME_FORMATTER.withZone(timeZone).parseMillis(value));
}
it used parseMillis(value)
. And parseMillis(value)
should be independent of timeZone
parameter, no?
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.
Specifically parseMillis
states:
* The parse will use the ISO chronology, and the default time zone.
* If the text contains a time zone string then that will be taken into account.
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.
the result of DATE_TIME_FORMATTER.withZone(timeZone).parseMillis(value)
depends on timeZone
when value
doesn't specify time zone.
No description provided.