----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71271/#review217349 -----------------------------------------------------------
Fix it, then Ship it! Thanks a lot for the patch. I have only one comment, otherwise it look good. common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveSqlDateTimeFormatter.java Lines 1025 (patched) <https://reviews.apache.org/r/71271/#comment304639> As we discussed, it would be enough to update the variable in an if statement when the temporalField is "IsoFields.WEEK_BASED_YEAR". In that case, there is no need for the updateVar method which is a bit confusing. - Marta Kuczora On Aug. 12, 2019, 10:59 a.m., Karen Coppage wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71271/ > ----------------------------------------------------------- > > (Updated Aug. 12, 2019, 10:59 a.m.) > > > Review request for hive and Marta Kuczora. > > > Bugs: HIVE-21580 > https://issues.apache.org/jira/browse/HIVE-21580 > > > Repository: hive-git > > > Description > ------- > > Enable Hive to parse the following datetime formats when any > combination/subset of these or previously implemented patterns is provided in > one string. Also catch combinations that conflict. > > IYYY > IYY > IY > I > IW > > > Diffs > ----- > > > common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveSqlDateTimeFormatter.java > 9443e8ec78 > > common/src/test/org/apache/hadoop/hive/common/format/datetime/TestHiveSqlDateTimeFormatter.java > ff41534fce > > > Diff: https://reviews.apache.org/r/71271/diff/1/ > > > Testing > ------- > > > Thanks, > > Karen Coppage > >
