> On Feb. 3, 2020, 9:12 a.m., Karen Coppage wrote: > > Thanks for the patch, looks good! Two ideas: > > 1. It would be nice to have a unit test that reads a date before October > > 1582, so it's clear that we're using the Proleptic Gregorian calendar. > > 2. ParquetTimestampUtils would be more readable if the big multipliers were > > declared as constants and/or in this format: e.g. 1_000_000. > > > > Thanks!
Thanks a lot Karen for the review. These are really good point. I fixed the numbers in the ParquetTimestampUtils and also added some test cases for dates before 1582. - Marta ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72074/#review219466 ----------------------------------------------------------- On Feb. 3, 2020, 12:31 p.m., Marta Kuczora wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72074/ > ----------------------------------------------------------- > > (Updated Feb. 3, 2020, 12:31 p.m.) > > > Review request for hive, Karen Coppage and Peter Vary. > > > Bugs: HIVE-21215 > https://issues.apache.org/jira/browse/HIVE-21215 > > > Repository: hive-git > > > Description > ------- > > Implemented the read path for Parquet INT64 timestamp. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/common/type/Timestamp.java > f2c1493f56 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java > d67b030648 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/ParquetTimestampUtils.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java > 519bd813e9 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedPrimitiveColumnReader.java > 2803baf90c > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/convert/TestETypeConverter.java > f6ee57140c > > > Diff: https://reviews.apache.org/r/72074/diff/2/ > > > Testing > ------- > > > Thanks, > > Marta Kuczora > >
