----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24713/#review50710 -----------------------------------------------------------
Looks good overall to me, one minor suggestion below. ql/src/java/org/apache/hadoop/hive/ql/metadata/VirtualColumn.java <https://reviews.apache.org/r/24713/#comment88575> Can we put VCols in a set for more efficiency, and also can we use Guava's Iterables to make this logic cleaner? - Szehon Ho On Aug. 14, 2014, 10:53 p.m., Mohit Sabharwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24713/ > ----------------------------------------------------------- > > (Updated Aug. 14, 2014, 10:53 p.m.) > > > Review request for hive. > > > Bugs: HIVE-7735 > https://issues.apache.org/jira/browse/HIVE-7735 > > > Repository: hive-git > > > Description > ------- > > HIVE-7735 : Implement Char, Varchar in ParquetSerDe > > - Since string, char and varchar are all represented as the same parquet > type (primitive type binary, original type utf8), this patch plumbs the > hive column types into ETypeConverter to distinguish between the three. > > - Removes Decimal related dead code in ArrayWritableObjectInspector, > (decimal is supported in Parquet SerDe) > > > Diffs > ----- > > data/files/parquet_types.txt 9d81c3c3130cb94ae2bc308d511b0e24a60d4b8e > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ArrayWritableGroupConverter.java > 582a5dfdaccaa25d46bfb515248eeb4bb84bedc5 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/DataWritableGroupConverter.java > 0e310fbfb748d5409ff3c0d8cd8327bec9988ecf > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/DataWritableRecordConverter.java > 7762afea4dda8cb4be4756eef43abec566ea8444 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java > 67ce15187a33d58fda7ff5b629339bd89d0e5e54 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java > 524a2937e39a4821a856c8e25b14633ade89ea49 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveSchemaConverter.java > 99901f0f57328db6fb2a260f7b7d76ded6f39558 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java > d6be4bdfc1502cf79c184726d88eb0bd94fb2b02 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ArrayWritableObjectInspector.java > 47bf69ce7cb6f474f9f48dd693a7915475a1d9cb > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java > e3e327c7b657cdd397dd2b4dddf40187c65ce901 > ql/src/java/org/apache/hadoop/hive/ql/metadata/VirtualColumn.java > 0637d46f2f7162c8d617c761e817dcf396fc94fe > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestHiveSchemaConverter.java > b87cf7449679a9b6da997010056e388fb3de9945 > ql/src/test/queries/clientnegative/parquet_char.q > 745a7867264e321c079d8146f60d14ae186bbc29 > ql/src/test/queries/clientnegative/parquet_varchar.q > 55825f76dc240c54ef451ceec12adee23f12b36c > ql/src/test/queries/clientpositive/parquet_types.q > cb0dcfdf2d637854a84b165f8565fcb683617696 > ql/src/test/results/clientnegative/parquet_char.q.out > eeaf33b3cca7ccc116fcec4bf11786f22d59c27f > ql/src/test/results/clientnegative/parquet_timestamp.q.out > 00973b7e1f6360ce830a8baa4b959491ccc87a9b > ql/src/test/results/clientnegative/parquet_varchar.q.out > c03a5b6bc991f12db66b7779c37b86f7a461ee1b > ql/src/test/results/clientpositive/parquet_types.q.out > dc6dc73479a8df3cd36bebfc8b5919893be33bcd > serde/src/java/org/apache/hadoop/hive/serde2/Deserializer.java > ade3b5f081eb71e5cf4e639aff8bff6447d68dfc > serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfo.java > e7f3f4837ab253a825a7210f56f595b2403e7385 > > Diff: https://reviews.apache.org/r/24713/diff/ > > > Testing > ------- > > - Added char, varchar types in parquet_types q-test. > - Added unit test for char, varchar in TestHiveSchemaConverter > - Removed char, varchar negative q-test files. > > > Thanks, > > Mohit Sabharwal > >
