> 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?
>
>
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.
- Xuefu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17061/#review33638
-----------------------------------------------------------
On Feb. 4, 2014, 8:29 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, 8:29 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
>
>
