You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Xuefu Zhang <xz...@cloudera.com> on 2014/02/04 00:53:49 UTC
Re: Review Request 17061: HIVE-5783 - Native Parquet Support in Hive
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17061/#review33473
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetInputFormat.java
<https://reviews.apache.org/r/17061/#comment62912>
This doesn't seem being used anywhere.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java
<https://reviews.apache.org/r/17061/#comment62911>
This doesn't seem necessary to be public.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java
<https://reviews.apache.org/r/17061/#comment62935>
If doubt exists, it's probably better to address it.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ProjectionPusher.java
<https://reviews.apache.org/r/17061/#comment62937>
Please remove if not used. Make it private otherwise. The same applies to all code.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ProjectionPusher.java
<https://reviews.apache.org/r/17061/#comment62943>
It's better to not to hard code those string consts here. They are probably defined somewhere.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ProjectionPusher.java
<https://reviews.apache.org/r/17061/#comment62951>
I don't understand what the conversion is about: string -> path -> uri -> path -> string?
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ProjectionPusher.java
<https://reviews.apache.org/r/17061/#comment62953>
Either put comments or log msg. Commented out code isn't comment.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ProjectionPusher.java
<https://reviews.apache.org/r/17061/#comment62954>
Same as above.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ProjectionPusher.java
<https://reviews.apache.org/r/17061/#comment62957>
Could you put more comments about what sort of refactoring? Can we log a JIRA for it also?
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ArrayWritableGroupConverter.java
<https://reviews.apache.org/r/17061/#comment62991>
The if ... else ... here doesn't seem terribly different. Please refeactor the code.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/DataWritableGroupConverter.java
<https://reviews.apache.org/r/17061/#comment62996>
It seems that for (int i = 0; i < selectFieldCount; i++) is better for this loop.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/DataWritableGroupConverter.java
<https://reviews.apache.org/r/17061/#comment62997>
Please remove the extra blank lines, which are not necessary. The same applies to all code changes.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java
<https://reviews.apache.org/r/17061/#comment62999>
Decimal treated as double? I don't think that's acceptable.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java
<https://reviews.apache.org/r/17061/#comment63000>
Please change to public static ...
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveSchemaConverter.java
<https://reviews.apache.org/r/17061/#comment63003>
Please change to "public static", across all code changes.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveSchemaConverter.java
<https://reviews.apache.org/r/17061/#comment63007>
Please wrap long lines. Applicable to all code changes.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java
<https://reviews.apache.org/r/17061/#comment63011>
1. private static?
2. use string constant instead of literal
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java
<https://reviews.apache.org/r/17061/#comment63021>
These string constant should be defined globally and referred here (and anywhere else).
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java
<https://reviews.apache.org/r/17061/#comment63020>
If not supposed to be call, it's better to throw an exception.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java
<https://reviews.apache.org/r/17061/#comment63022>
Same as above.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java
<https://reviews.apache.org/r/17061/#comment63023>
long too long
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java
<https://reviews.apache.org/r/17061/#comment63024>
long lines.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java
<https://reviews.apache.org/r/17061/#comment63030>
I don't understand why Hive would inspect an inspected result.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ArrayWritableObjectInspector.java
<https://reviews.apache.org/r/17061/#comment63029>
I don't understand why Hive would inspect an inspected result.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/DeepParquetHiveMapInspector.java
<https://reviews.apache.org/r/17061/#comment63035>
Again, why?
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveArrayInspector.java
<https://reviews.apache.org/r/17061/#comment63038>
Same here too.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java
<https://reviews.apache.org/r/17061/#comment63039>
final?
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java
<https://reviews.apache.org/r/17061/#comment63040>
Please remove extra blank lines.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java
<https://reviews.apache.org/r/17061/#comment63041>
String constants?
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java
<https://reviews.apache.org/r/17061/#comment63043>
for(int i = 0; i < fields.size(); i++) seems more appropriate.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java
<https://reviews.apache.org/r/17061/#comment63044>
Please remove extrac blank lines.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/StandardParquetHiveMapInspector.java
<https://reviews.apache.org/r/17061/#comment63045>
Again, why? We don't code against wrong assumption.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/writable/BinaryWritable.java
<https://reviews.apache.org/r/17061/#comment63046>
Do we have a followup JIRA?
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriteSupport.java
<https://reviews.apache.org/r/17061/#comment63048>
Move this to the top?
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java
<https://reviews.apache.org/r/17061/#comment63049>
remove extrac blank lines.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java
<https://reviews.apache.org/r/17061/#comment63050>
remove extrac blank lines.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java
<https://reviews.apache.org/r/17061/#comment63052>
We need to be consistent for decimal. I see code dealing with it, and also code that disallows it.
ql/src/java/parquet/hive/DeprecatedParquetInputFormat.java
<https://reviews.apache.org/r/17061/#comment63054>
I don't quite get how this provides backward compatibility. Please explain.
ql/src/java/parquet/hive/DeprecatedParquetOutputFormat.java
<https://reviews.apache.org/r/17061/#comment63055>
Same as above.
- Xuefu Zhang
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
>
>
Re: Review Request 17061: HIVE-5783 - Native Parquet Support in Hive
Posted by Brock Noland <br...@cloudera.com>.
> On Feb. 3, 2014, 11:53 p.m., Xuefu Zhang wrote:
> > ql/src/java/parquet/hive/DeprecatedParquetInputFormat.java, line 22
> > <https://reviews.apache.org/r/17061/diff/5/?file=454363#file454363line22>
> >
> > I don't quite get how this provides backward compatibility. Please explain.
The issue is that users til now specified the create table statement below which puts those class names in the metastore. The same is true for all parquet.hive.* classes included in this change.
CREATE TABLE parquet_test (column1 string)
ROW FORMAT SERDE 'parquet.hive.serde.ParquetHiveSerDe'
STORED AS
INPUTFORMAT 'parquet.hive.DeprecatedParquetInputFormat'
OUTPUTFORMAT 'parquet.hive.DeprecatedParquetOutputFormat';
- 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
>
>
Re: Review Request 17061: HIVE-5783 - Native Parquet Support in Hive
Posted by Brock Noland <br...@cloudera.com>.
> 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
>
>
Re: Review Request 17061: HIVE-5783 - Native Parquet Support in Hive
Posted by Brock Noland <br...@cloudera.com>.
> On Feb. 3, 2014, 11:53 p.m., Xuefu Zhang wrote:
> > ql/src/java/parquet/hive/DeprecatedParquetInputFormat.java, line 22
> > <https://reviews.apache.org/r/17061/diff/5/?file=454363#file454363line22>
> >
> > I don't quite get how this provides backward compatibility. Please explain.
>
> Brock Noland wrote:
> The issue is that users til now specified the create table statement below which puts those class names in the metastore. The same is true for all parquet.hive.* classes included in this change.
>
> CREATE TABLE parquet_test (column1 string)
> ROW FORMAT SERDE 'parquet.hive.serde.ParquetHiveSerDe'
> STORED AS
> INPUTFORMAT 'parquet.hive.DeprecatedParquetInputFormat'
> OUTPUTFORMAT 'parquet.hive.DeprecatedParquetOutputFormat';
>
>
> Xuefu Zhang wrote:
> OKay. These are interesting class names acutually used.
The reason I believe is that a long time ago someone named the "org.apache.hadoop.mapred" version of the LZO inputformat "Deprecated...InputFormat". That has percolated to many other projects. After some discussions with the Parquet folks we created MapredParquetInputFormat to replace DepcreatedParquetInputFormat.
- Brock
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17061/#review33473
-----------------------------------------------------------
On Feb. 4, 2014, 4:52 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, 4:52 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/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
>
>
Re: Review Request 17061: HIVE-5783 - Native Parquet Support in Hive
Posted by Xuefu Zhang <xz...@cloudera.com>.
> On Feb. 3, 2014, 11:53 p.m., Xuefu Zhang wrote:
> > ql/src/java/parquet/hive/DeprecatedParquetInputFormat.java, line 22
> > <https://reviews.apache.org/r/17061/diff/5/?file=454363#file454363line22>
> >
> > I don't quite get how this provides backward compatibility. Please explain.
>
> Brock Noland wrote:
> The issue is that users til now specified the create table statement below which puts those class names in the metastore. The same is true for all parquet.hive.* classes included in this change.
>
> CREATE TABLE parquet_test (column1 string)
> ROW FORMAT SERDE 'parquet.hive.serde.ParquetHiveSerDe'
> STORED AS
> INPUTFORMAT 'parquet.hive.DeprecatedParquetInputFormat'
> OUTPUTFORMAT 'parquet.hive.DeprecatedParquetOutputFormat';
>
OKay. These are interesting class names acutually used.
- Xuefu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17061/#review33473
-----------------------------------------------------------
On Feb. 4, 2014, 4:52 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, 4:52 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/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
>
>