[
https://issues.apache.org/jira/browse/HIVE-22685?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17015097#comment-17015097
]
David Mollitor commented on HIVE-22685:
---------------------------------------
[~klcopp] Thanks for the review.
Yes. I see now that it needs to be serialized. Thanks for bringing that to my
attention as I was surprised to see that this is a requirement. OK. Yes, Java
Optional is not Serializeable. While technically they were not intended to be
class variables, I personally think they make a lot of sense as instance
variables because of their idiot-proof-ness. If they are used as intended...
only in Getter/Setter then it adds a lot of boilerplate code that is subject to
fat-fingers. Most importantly, I tend to respect what Google does above all
else:
Their Optional implementation is serializeable so that it can be used as an
instance variable.
[https://guava.dev/releases/19.0/api/docs/com/google/common/base/Optional.html]
But yes, in this case, since it does need to be serializeable, we can use the
Guava optional (my preference) or use native null values.
I still very much encourage that we keep the ability to pass in a
{{LocalDateTime}}. Maybe we can lessen the scope to default package and add
the VisibleForTesting annotation. I think it's not ideal that the values in
the test change every time the test is ran. Sometimes its unavoidable, but in
this case passing in a known value is not too intrusive. If the test fails (as
in this case) the developer has no way to know what the value was at that exact
moment that caused it to fail. Passing in a known value, that does not change,
is ideal for debugging and testing.
> TestHiveSqlDateTimeFormatter Now Broken with New Year 2020
> ----------------------------------------------------------
>
> Key: HIVE-22685
> URL: https://issues.apache.org/jira/browse/HIVE-22685
> Project: Hive
> Issue Type: Bug
> Reporter: David Mollitor
> Assignee: David Mollitor
> Priority: Major
> Attachments: HIVE-22685.1.patch, HIVE-22685.2.patch
>
>
> Unit test is now broken.... (n)(n):(
> {code:java}
> //Tests for these patterns would need changing every decade if done in
> the above way.
> //Thursday of the first week in an ISO year always matches the Gregorian
> year.
> checkParseTimestampIso("IY-IW-ID", "0-01-04", "iw, yyyy", "01, " +
> thisYearString.substring(0, 3) + "0");
> checkParseTimestampIso("I-IW-ID", "0-01-04", "iw, yyyy", "01, " +
> thisYearString.substring(0, 3) + "0");
> {code}
> {code}
> org.junit.ComparisonFailure: expected:<01, 20[1]0> but was:<01, 20[2]0>
> at org.junit.Assert.assertEquals(Assert.java:115)
> at org.junit.Assert.assertEquals(Assert.java:144)
> at
> org.apache.hadoop.hive.common.format.datetime.TestHiveSqlDateTimeFormatter.checkParseTimestampIso(TestHiveSqlDateTimeFormatter.java:313)
> at
> org.apache.hadoop.hive.common.format.datetime.TestHiveSqlDateTimeFormatter.testParseTimestamp(TestHiveSqlDateTimeFormatter.java:287)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:498)
> at
> org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
> at
> org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
> at
> org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
> at
> org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
> at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
