hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Brock Noland" <br...@cloudera.com>
Subject Re: Review Request 17061: HIVE-5783 - Native Parquet Support in Hive
Date Tue, 04 Feb 2014 16:24:45 GMT


> On Feb. 3, 2014, 11:53 p.m., Xuefu Zhang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ProjectionPusher.java, line 102
> > <https://reviews.apache.org/r/17061/diff/5/?file=454335#file454335line102>
> >
> >     Same as above.

Hive hasn't always been good at handling schemes and URI's in general. For example look at
the work to convert the core from URI/String to Path. I think we need to understand why this
code does this and then clean it up. I added this as work to take forward in HIVE-6366.


> On Feb. 3, 2014, 11:53 p.m., Xuefu Zhang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java, line
98
> > <https://reviews.apache.org/r/17061/diff/5/?file=454339#file454339line98>
> >
> >     Decimal treated as double? I don't think that's acceptable.

Decimal support is not fully implemented. We can take this forward in HIVE-6367.


> On Feb. 3, 2014, 11:53 p.m., Xuefu Zhang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java,
line 79
> > <https://reviews.apache.org/r/17061/diff/5/?file=454344#file454344line79>
> >
> >     I don't understand why Hive would inspect an inspected result.

Agreed. I don't see any harm with leaving this in for now and resolving in HIVE-6366.


> On Feb. 3, 2014, 11:53 p.m., Xuefu Zhang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/writable/BinaryWritable.java, line
28
> > <https://reviews.apache.org/r/17061/diff/5/?file=454355#file454355line28>
> >
> >     Do we have a followup JIRA?

Yep, will be covered in HIVE-6366


> On Feb. 3, 2014, 11:53 p.m., Xuefu Zhang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java,
line 153
> > <https://reviews.apache.org/r/17061/diff/5/?file=454357#file454357line153>
> >
> >     We need to be consistent for decimal. I see code dealing with it, and also code
that disallows it.

I agree 100%. We'll take this forward in HIVE-6367.


> On Feb. 3, 2014, 11:53 p.m., Xuefu Zhang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ProjectionPusher.java, line 81
> > <https://reviews.apache.org/r/17061/diff/5/?file=454335#file454335line81>
> >
> >     I don't understand what the conversion is about: string -> path -> uri
-> path -> string?

I think it's "only" string -> path -> uri -> string. I added a comment about this,
but we plan on taking it forward in HIVE-6366 as I believe there is more analysis to be done
there.


- Brock


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17061/#review33473
-----------------------------------------------------------


On Jan. 30, 2014, 2:48 p.m., Brock Noland wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17061/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2014, 2:48 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/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/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 010e04f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 538b2b0 
>   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/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