You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Adam Szita <sz...@cloudera.com> on 2017/05/05 08:11:56 UTC
Review Request 59020: Support Parquet through HCatalog
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59020/
-----------------------------------------------------------
Review request for hive, Aihua Xu and Sergio Pena.
Bugs: HIVE-8838
https://issues.apache.org/jira/browse/HIVE-8838
Repository: hive-git
Description
-------
Adding support for HCatalog to write tables stored in Parquet format
Diffs
-----
hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/FileRecordWriterContainer.java b2abc5fbb3670893415354552239d67d072459ed
hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/SpecialCases.java 60af5c0bf397273fb820f0ee31e578745dbc200f
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderComplexSchema.java 4c686fec596d39d41d458bc3ea2753877bd9df98
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderEncryption.java ad11eab1b7e67541b56e90e4a85ba37b41a4db92
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatStorerMulti.java 918332ddfda58306707d326f8668b2c223110a29
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatLoader.java 6cd382145b55d6b85fc3366faeaba2aaef65ab04
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatStorer.java 6dfdc04954dd0b110b1a7194e69468b5dc2f842e
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java a7bb5eedbb99f3cea4601b9fce9a0ad3461567d0
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java b339cc4347eea143dca2f6d98f9aa7777afdc427
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriteSupport.java 71a78cf040667bf14b6c720373e4acd102da19f4
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/ParquetRecordWriterWrapper.java c021dafa480e65d7c0c19a5a85988464112468cb
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapredParquetOutputFormat.java ec85b5df0f95cbd45b87259346ae9c1e5aa604a4
Diff: https://reviews.apache.org/r/59020/diff/1/
Testing
-------
Tested on cluster, and re-enabled previously disabled tests in HCatalog (for Parquet) that were failing (this adds ~40 tests to be run)
Thanks,
Adam Szita
Re: Review Request 59020: Support Parquet through HCatalog
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59020/#review180306
-----------------------------------------------------------
Ship it!
Ship It!
- Sergio Pena
On July 12, 2017, 2:57 p.m., Adam Szita wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59020/
> -----------------------------------------------------------
>
> (Updated July 12, 2017, 2:57 p.m.)
>
>
> Review request for hive, Aihua Xu and Sergio Pena.
>
>
> Bugs: HIVE-8838
> https://issues.apache.org/jira/browse/HIVE-8838
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Adding support for HCatalog to write tables stored in Parquet format
>
>
> Diffs
> -----
>
> hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/FileRecordWriterContainer.java b2abc5fbb3670893415354552239d67d072459ed
> hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/SpecialCases.java 60af5c0bf397273fb820f0ee31e578745dbc200f
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderComplexSchema.java 4c686fec596d39d41d458bc3ea2753877bd9df98
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderEncryption.java ad11eab1b7e67541b56e90e4a85ba37b41a4db92
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatStorerMulti.java 918332ddfda58306707d326f8668b2c223110a29
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatLoader.java 6cd382145b55d6b85fc3366faeaba2aaef65ab04
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatStorer.java 6dfdc04954dd0b110b1a7194e69468b5dc2f842e
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java 379a9135d9c631b2f473976b00f3dc87f9fec0c4
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/ParquetRecordWriterWrapper.java c021dafa480e65d7c0c19a5a85988464112468cb
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapredParquetOutputFormat.java ec85b5df0f95cbd45b87259346ae9c1e5aa604a4
>
>
> Diff: https://reviews.apache.org/r/59020/diff/2/
>
>
> Testing
> -------
>
> Tested on cluster, and re-enabled previously disabled tests in HCatalog (for Parquet) that were failing (this adds ~40 tests to be run)
>
>
> Thanks,
>
> Adam Szita
>
>
Re: Review Request 59020: Support Parquet through HCatalog
Posted by Adam Szita <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59020/
-----------------------------------------------------------
(Updated July 12, 2017, 2:57 p.m.)
Review request for hive, Aihua Xu and Sergio Pena.
Changes
-------
Attached rebased patch and addressed comments
Bugs: HIVE-8838
https://issues.apache.org/jira/browse/HIVE-8838
Repository: hive-git
Description
-------
Adding support for HCatalog to write tables stored in Parquet format
Diffs (updated)
-----
hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/FileRecordWriterContainer.java b2abc5fbb3670893415354552239d67d072459ed
hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/SpecialCases.java 60af5c0bf397273fb820f0ee31e578745dbc200f
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderComplexSchema.java 4c686fec596d39d41d458bc3ea2753877bd9df98
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderEncryption.java ad11eab1b7e67541b56e90e4a85ba37b41a4db92
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatStorerMulti.java 918332ddfda58306707d326f8668b2c223110a29
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatLoader.java 6cd382145b55d6b85fc3366faeaba2aaef65ab04
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatStorer.java 6dfdc04954dd0b110b1a7194e69468b5dc2f842e
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java 379a9135d9c631b2f473976b00f3dc87f9fec0c4
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/ParquetRecordWriterWrapper.java c021dafa480e65d7c0c19a5a85988464112468cb
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapredParquetOutputFormat.java ec85b5df0f95cbd45b87259346ae9c1e5aa604a4
Diff: https://reviews.apache.org/r/59020/diff/2/
Changes: https://reviews.apache.org/r/59020/diff/1-2/
Testing
-------
Tested on cluster, and re-enabled previously disabled tests in HCatalog (for Parquet) that were failing (this adds ~40 tests to be run)
Thanks,
Adam Szita
Re: Review Request 59020: Support Parquet through HCatalog
Posted by Adam Szita <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59020/
-----------------------------------------------------------
(Updated July 12, 2017, 2:53 p.m.)
Review request for hive, Aihua Xu and Sergio Pena.
Changes
-------
Attached rebased patch (due to timestamp issue revert) + addressed comments
Bugs: HIVE-8838
https://issues.apache.org/jira/browse/HIVE-8838
Repository: hive-git
Description
-------
Adding support for HCatalog to write tables stored in Parquet format
Diffs (updated)
-----
hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/FileRecordWriterContainer.java b2abc5fbb3670893415354552239d67d072459ed
hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/SpecialCases.java 60af5c0bf397273fb820f0ee31e578745dbc200f
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderComplexSchema.java 4c686fec596d39d41d458bc3ea2753877bd9df98
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderEncryption.java ad11eab1b7e67541b56e90e4a85ba37b41a4db92
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatStorerMulti.java 918332ddfda58306707d326f8668b2c223110a29
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatLoader.java 6cd382145b55d6b85fc3366faeaba2aaef65ab04
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatStorer.java 6dfdc04954dd0b110b1a7194e69468b5dc2f842e
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java 379a9135d9c631b2f473976b00f3dc87f9fec0c4
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/ParquetRecordWriterWrapper.java c021dafa480e65d7c0c19a5a85988464112468cb
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapredParquetOutputFormat.java ec85b5df0f95cbd45b87259346ae9c1e5aa604a4
Diff: https://reviews.apache.org/r/59020/diff/2/
Changes: https://reviews.apache.org/r/59020/diff/1-2/
Testing
-------
Tested on cluster, and re-enabled previously disabled tests in HCatalog (for Parquet) that were failing (this adds ~40 tests to be run)
Thanks,
Adam Szita
Re: Review Request 59020: Support Parquet through HCatalog
Posted by Adam Szita <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59020/
-----------------------------------------------------------
(Updated July 12, 2017, 2:54 p.m.)
Review request for hive, Aihua Xu and Sergio Pena.
Changes
-------
Attached rebased patch (due to timestamp issue revert) + addressed comments
Bugs: HIVE-8838
https://issues.apache.org/jira/browse/HIVE-8838
Repository: hive-git
Description
-------
Adding support for HCatalog to write tables stored in Parquet format
Diffs (updated)
-----
hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/FileRecordWriterContainer.java b2abc5fbb3670893415354552239d67d072459ed
hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/SpecialCases.java 60af5c0bf397273fb820f0ee31e578745dbc200f
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderComplexSchema.java 4c686fec596d39d41d458bc3ea2753877bd9df98
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderEncryption.java ad11eab1b7e67541b56e90e4a85ba37b41a4db92
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatStorerMulti.java 918332ddfda58306707d326f8668b2c223110a29
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatLoader.java 6cd382145b55d6b85fc3366faeaba2aaef65ab04
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatStorer.java 6dfdc04954dd0b110b1a7194e69468b5dc2f842e
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java 379a9135d9c631b2f473976b00f3dc87f9fec0c4
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/ParquetRecordWriterWrapper.java c021dafa480e65d7c0c19a5a85988464112468cb
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapredParquetOutputFormat.java ec85b5df0f95cbd45b87259346ae9c1e5aa604a4
Diff: https://reviews.apache.org/r/59020/diff/2/
Changes: https://reviews.apache.org/r/59020/diff/1-2/
Testing
-------
Tested on cluster, and re-enabled previously disabled tests in HCatalog (for Parquet) that were failing (this adds ~40 tests to be run)
Thanks,
Adam Szita
Re: Review Request 59020: Support Parquet through HCatalog
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59020/#review180234
-----------------------------------------------------------
Fix it, then Ship it!
The patch looks good Adam. Please just add the comments and remove the unused imports.
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatStorerMulti.java
Lines 46-49 (original), 46-49 (patched)
<https://reviews.apache.org/r/59020/#comment255324>
There are a few unused imports. Remove them.
hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatStorer.java
Lines 21-28 (original), 21-28 (patched)
<https://reviews.apache.org/r/59020/#comment255323>
There are a few unused imports. Remove them.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java
Line 76 (original), 76 (patched)
<https://reviews.apache.org/r/59020/#comment255325>
Could you add a javadoc comment about what parameters are expected on the JobConf object?
I see that this constructor won't work unless the PARQUET_HIVE_SCHEMA is set by the caller (HCat is doing in on the SpecialCases), and we have to leave that comment about the assumptions.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java
Lines 24 (patched)
<https://reviews.apache.org/r/59020/#comment255309>
I like to avoid short words on method names. What bout just using isParquetProperty() instead? The class name already implies this is meant for parquet tables.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/ParquetRecordWriterWrapper.java
Lines 87 (patched)
<https://reviews.apache.org/r/59020/#comment255312>
Same thing with the short words on method names.
However, makes more sense to use getParquetProperties(), what do you think? Table properties are set through the Table, but jobConf can bring other parquet properties that were set in hive-site.xml, right?
- Sergio Pena
On May 5, 2017, 8:11 a.m., Adam Szita wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59020/
> -----------------------------------------------------------
>
> (Updated May 5, 2017, 8:11 a.m.)
>
>
> Review request for hive, Aihua Xu and Sergio Pena.
>
>
> Bugs: HIVE-8838
> https://issues.apache.org/jira/browse/HIVE-8838
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Adding support for HCatalog to write tables stored in Parquet format
>
>
> Diffs
> -----
>
> hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/FileRecordWriterContainer.java b2abc5fbb3670893415354552239d67d072459ed
> hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/SpecialCases.java 60af5c0bf397273fb820f0ee31e578745dbc200f
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderComplexSchema.java 4c686fec596d39d41d458bc3ea2753877bd9df98
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderEncryption.java ad11eab1b7e67541b56e90e4a85ba37b41a4db92
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatStorerMulti.java 918332ddfda58306707d326f8668b2c223110a29
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatLoader.java 6cd382145b55d6b85fc3366faeaba2aaef65ab04
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatStorer.java 6dfdc04954dd0b110b1a7194e69468b5dc2f842e
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java a7bb5eedbb99f3cea4601b9fce9a0ad3461567d0
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java b339cc4347eea143dca2f6d98f9aa7777afdc427
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriteSupport.java 71a78cf040667bf14b6c720373e4acd102da19f4
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/ParquetRecordWriterWrapper.java c021dafa480e65d7c0c19a5a85988464112468cb
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapredParquetOutputFormat.java ec85b5df0f95cbd45b87259346ae9c1e5aa604a4
>
>
> Diff: https://reviews.apache.org/r/59020/diff/1/
>
>
> Testing
> -------
>
> Tested on cluster, and re-enabled previously disabled tests in HCatalog (for Parquet) that were failing (this adds ~40 tests to be run)
>
>
> Thanks,
>
> Adam Szita
>
>
Re: Review Request 59020: Support Parquet through HCatalog
Posted by Aihua Xu <ax...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59020/#review174039
-----------------------------------------------------------
hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/FileRecordWriterContainer.java
Lines 30 (patched)
<https://reviews.apache.org/r/59020/#comment247146>
This change looks good to me.
Sergio, can you also help review the patch since you are more faimilar with Parquet?
- Aihua Xu
On May 5, 2017, 8:11 a.m., Adam Szita wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59020/
> -----------------------------------------------------------
>
> (Updated May 5, 2017, 8:11 a.m.)
>
>
> Review request for hive, Aihua Xu and Sergio Pena.
>
>
> Bugs: HIVE-8838
> https://issues.apache.org/jira/browse/HIVE-8838
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Adding support for HCatalog to write tables stored in Parquet format
>
>
> Diffs
> -----
>
> hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/FileRecordWriterContainer.java b2abc5fbb3670893415354552239d67d072459ed
> hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/SpecialCases.java 60af5c0bf397273fb820f0ee31e578745dbc200f
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderComplexSchema.java 4c686fec596d39d41d458bc3ea2753877bd9df98
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatLoaderEncryption.java ad11eab1b7e67541b56e90e4a85ba37b41a4db92
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestHCatStorerMulti.java 918332ddfda58306707d326f8668b2c223110a29
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatLoader.java 6cd382145b55d6b85fc3366faeaba2aaef65ab04
> hcatalog/hcatalog-pig-adapter/src/test/java/org/apache/hive/hcatalog/pig/TestParquetHCatStorer.java 6dfdc04954dd0b110b1a7194e69468b5dc2f842e
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java a7bb5eedbb99f3cea4601b9fce9a0ad3461567d0
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java b339cc4347eea143dca2f6d98f9aa7777afdc427
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriteSupport.java 71a78cf040667bf14b6c720373e4acd102da19f4
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/ParquetRecordWriterWrapper.java c021dafa480e65d7c0c19a5a85988464112468cb
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapredParquetOutputFormat.java ec85b5df0f95cbd45b87259346ae9c1e5aa604a4
>
>
> Diff: https://reviews.apache.org/r/59020/diff/1/
>
>
> Testing
> -------
>
> Tested on cluster, and re-enabled previously disabled tests in HCatalog (for Parquet) that were failing (this adds ~40 tests to be run)
>
>
> Thanks,
>
> Adam Szita
>
>