> On Feb. 4, 2014, 9:44 p.m., Xuefu Zhang wrote: > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ArrayWritableGroupConverter.java, > > line 42 > > <https://reviews.apache.org/r/17061/diff/8/?file=469225#file469225line42> > > > > I'd rather throw an exception if (count != 1 && count != 2) then using > > assert here. > > Brock Noland wrote: > Maybe I misunderstood your earlier comment about this code "The if ... > else ... here doesn't seem terribly different. Please refeactor the code." as > the earlier code seems like what you are suggesting. Can you psuedo code what > you'd like to see here? > > > > Xuefu Zhang wrote: > Sorry for the confusion. My earlier comments meant to say that the two > cases, count = 1 or count = 2, seemed very similar, except setting isMap > either true or false.Thus, it seemed that the two block could be combined. > > int count = groupType.getFieldCount(); > if (count != 1 && count != 2) { > throw new Exception .... > } > > isMap = count == 2; > converters = new Converter[count]; > for (int i = 0; i < count; i++) { > converters[i] = getConverterFromDescription(groupType.getType(count > - 1), i, this); > } > > Now I realize that the code is simpler yet a little harder to understand. > So, it's up to you. > > However, I think throwing an exception is better than an assertion > regardless.
Ok...thank you for clarifying! I when with your approach. - Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17061/#review33638 ----------------------------------------------------------- On Feb. 4, 2014, 10:36 p.m., Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17061/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2014, 10:36 p.m.) > > > Review request for hive. > > > Bugs: HIVE-5783 > https://issues.apache.org/jira/browse/HIVE-5783 > > > Repository: hive-git > > > Description > ------- > > Adds native parquet support hive > > > Diffs > ----- > > data/files/parquet_create.txt PRE-CREATION > data/files/parquet_partitioned.txt PRE-CREATION > pom.xml 41f5337 > ql/pom.xml 7087a4c > ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetInputFormat.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ProjectionPusher.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ArrayWritableGroupConverter.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/DataWritableGroupConverter.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/DataWritableRecordConverter.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveSchemaConverter.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ArrayWritableObjectInspector.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/DeepParquetHiveMapInspector.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveArrayInspector.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/StandardParquetHiveMapInspector.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/primitive/ParquetByteInspector.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/primitive/ParquetPrimitiveInspectorFactory.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/primitive/ParquetShortInspector.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/primitive/ParquetStringInspector.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/writable/BigDecimalWritable.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/writable/BinaryWritable.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriteSupport.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/ParquetRecordWriterWrapper.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/metadata/VirtualColumn.java 2bc7e86 > ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java > 13d0a56 > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g f83c15d > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 17f3552 > ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 538b2b0 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 719b496 > > ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryImpl.java > 4ac8f07 > ql/src/java/parquet/hive/DeprecatedParquetInputFormat.java PRE-CREATION > ql/src/java/parquet/hive/DeprecatedParquetOutputFormat.java PRE-CREATION > ql/src/java/parquet/hive/MapredParquetInputFormat.java PRE-CREATION > ql/src/java/parquet/hive/MapredParquetOutputFormat.java PRE-CREATION > ql/src/java/parquet/hive/serde/ParquetHiveSerDe.java PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestHiveSchemaConverter.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapredParquetInputFormat.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapredParquetOutputFormat.java > PRE-CREATION > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetSerDe.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestAbstractParquetMapInspector.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestDeepParquetHiveMapInspector.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetHiveArrayInspector.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestStandardParquetHiveMapInspector.java > PRE-CREATION > ql/src/test/queries/clientpositive/parquet_create.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_partitioned.q PRE-CREATION > ql/src/test/results/clientnegative/authorization_invalid_priv_v1.q.out > 431f16e > ql/src/test/results/clientpositive/parquet_create.q.out PRE-CREATION > ql/src/test/results/clientpositive/parquet_partitioned.q.out PRE-CREATION > > Diff: https://reviews.apache.org/r/17061/diff/ > > > Testing > ------- > > > Thanks, > > Brock Noland > >
