hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Xuefu Zhang" <xzh...@cloudera.com>
Subject Re: Review Request 17061: HIVE-5783 - Native Parquet Support in Hive
Date Tue, 04 Feb 2014 22:21:14 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message