You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Sergio Pena <se...@cloudera.com> on 2015/02/07 00:34:38 UTC

Review Request 30750: HIVE-9605 Remove parquet nested objects from wrapper writable objects

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

Review request for hive, Ryan Blue, cheng xu, and Dong Chen.


Bugs: HIVE-9605
    https://issues.apache.org/jira/browse/HIVE-9605


Repository: hive-git


Description
-------

Remove wrapper object from parquet nested types (map/array)


Diffs
-----

  itests/hive-jmh/src/main/java/org/apache/hive/benchmark/storage/ColumnarStorageBench.java d335716be3d286a1b9221dcbd8ccd799f4c6dc66 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 6621a8768953a9bef54e7a144ae045abcc32f458 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java fdea782167d63593f6cbde5e7154d771761757f7 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java 62c61fc7502f24e6a032076f384b5a946c1cc9a6 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/DeepParquetHiveMapInspector.java d38c64192e01371c0c98b339113348d2e52cedc3 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveArrayInspector.java 53ca31d0b516c4a941e048e98e7f8f763752c436 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/StandardParquetHiveMapInspector.java 5aa14482899fed5711b40c5554b056d07818afb5 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapStructures.java ca4805082fd717d15ed41ca15a730e19da267c8a 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestAbstractParquetMapInspector.java ef05150494027ddd70790dcf26b772ebc4cd2b8b 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestDeepParquetHiveMapInspector.java 8646ff4d3413d7d642e2559e1a485d77472b156a 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetHiveArrayInspector.java f3a24af2e5f4eeb24e1e286ada19fc9592daacb6 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestStandardParquetHiveMapInspector.java 278419f73b311322dcf3c70abb340bf63d8a4337 

Diff: https://reviews.apache.org/r/30750/diff/


Testing
-------


Thanks,

Sergio Pena


Re: Review Request 30750: HIVE-9605 Remove parquet nested objects from wrapper writable objects

Posted by Sergio Pena <se...@cloudera.com>.

> On Feb. 10, 2015, 10:40 p.m., Ryan Blue wrote:
> > There are quite a few non-functional changes: renaming fields (mapArray => mapContainer), moving imports, and white-space changes. I'm not sure what Hive's policy on that is, but I generally try to avoid it because 1) it make conflicts between patches much more likely and 2) it makes it harder to track down bugs later using things like git blame.
> > 
> > Conflicts are bad when we cherry-pick a lot of commits, so at least the white-space and import moves are a larger risk than you'd think. Renaming fields is up to you since you're the maintainer.

Thanks Ryan.

I had my IDE configured for auto-import, and I did not notice all this changes.
I rellocate the imports.

Regarding the names, I change them because I removed the wrapList(), and those names were related to the wrapper object.


- Sergio


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


On Feb. 10, 2015, 9:25 p.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30750/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2015, 9:25 p.m.)
> 
> 
> Review request for hive, Ryan Blue, cheng xu, and Dong Chen.
> 
> 
> Bugs: HIVE-9605
>     https://issues.apache.org/jira/browse/HIVE-9605
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Remove wrapper object from parquet nested types (map/array)
> 
> 
> Diffs
> -----
> 
>   itests/hive-jmh/src/main/java/org/apache/hive/benchmark/storage/ColumnarStorageBench.java 61c2eb418e4ad317a603e87240295f9396d1d5b5 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 6621a8768953a9bef54e7a144ae045abcc32f458 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java 4809f9b5882ae409159b422c08c665aa24f796d8 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java fdea782167d63593f6cbde5e7154d771761757f7 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java 62c61fc7502f24e6a032076f384b5a946c1cc9a6 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/DeepParquetHiveMapInspector.java d38c64192e01371c0c98b339113348d2e52cedc3 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveArrayInspector.java 53ca31d0b516c4a941e048e98e7f8f763752c436 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/StandardParquetHiveMapInspector.java 5aa14482899fed5711b40c5554b056d07818afb5 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestArrayCompatibility.java f7f3e57c1607dec3e8dc3a29e819d3aadc65155a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapStructures.java ca4805082fd717d15ed41ca15a730e19da267c8a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetSerDe.java 007a665529857bcec612f638a157aa5043562a15 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestAbstractParquetMapInspector.java ef05150494027ddd70790dcf26b772ebc4cd2b8b 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestDeepParquetHiveMapInspector.java 8646ff4d3413d7d642e2559e1a485d77472b156a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetHiveArrayInspector.java f3a24af2e5f4eeb24e1e286ada19fc9592daacb6 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestStandardParquetHiveMapInspector.java 278419f73b311322dcf3c70abb340bf63d8a4337 
> 
> Diff: https://reviews.apache.org/r/30750/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>


Re: Review Request 30750: HIVE-9605 Remove parquet nested objects from wrapper writable objects

Posted by Ryan Blue <bl...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30750/#review71865
-----------------------------------------------------------

Ship it!


There are quite a few non-functional changes: renaming fields (mapArray => mapContainer), moving imports, and white-space changes. I'm not sure what Hive's policy on that is, but I generally try to avoid it because 1) it make conflicts between patches much more likely and 2) it makes it harder to track down bugs later using things like git blame.

Conflicts are bad when we cherry-pick a lot of commits, so at least the white-space and import moves are a larger risk than you'd think. Renaming fields is up to you since you're the maintainer.


ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java
<https://reviews.apache.org/r/30750/#comment117743>

    Nit: relocating import statements


- Ryan Blue


On Feb. 10, 2015, 1:25 p.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30750/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2015, 1:25 p.m.)
> 
> 
> Review request for hive, Ryan Blue, cheng xu, and Dong Chen.
> 
> 
> Bugs: HIVE-9605
>     https://issues.apache.org/jira/browse/HIVE-9605
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Remove wrapper object from parquet nested types (map/array)
> 
> 
> Diffs
> -----
> 
>   itests/hive-jmh/src/main/java/org/apache/hive/benchmark/storage/ColumnarStorageBench.java 61c2eb418e4ad317a603e87240295f9396d1d5b5 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 6621a8768953a9bef54e7a144ae045abcc32f458 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java 4809f9b5882ae409159b422c08c665aa24f796d8 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java fdea782167d63593f6cbde5e7154d771761757f7 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java 62c61fc7502f24e6a032076f384b5a946c1cc9a6 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/DeepParquetHiveMapInspector.java d38c64192e01371c0c98b339113348d2e52cedc3 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveArrayInspector.java 53ca31d0b516c4a941e048e98e7f8f763752c436 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/StandardParquetHiveMapInspector.java 5aa14482899fed5711b40c5554b056d07818afb5 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestArrayCompatibility.java f7f3e57c1607dec3e8dc3a29e819d3aadc65155a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapStructures.java ca4805082fd717d15ed41ca15a730e19da267c8a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetSerDe.java 007a665529857bcec612f638a157aa5043562a15 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestAbstractParquetMapInspector.java ef05150494027ddd70790dcf26b772ebc4cd2b8b 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestDeepParquetHiveMapInspector.java 8646ff4d3413d7d642e2559e1a485d77472b156a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetHiveArrayInspector.java f3a24af2e5f4eeb24e1e286ada19fc9592daacb6 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestStandardParquetHiveMapInspector.java 278419f73b311322dcf3c70abb340bf63d8a4337 
> 
> Diff: https://reviews.apache.org/r/30750/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>


Re: Review Request 30750: HIVE-9605 Remove parquet nested objects from wrapper writable objects

Posted by Ryan Blue <bl...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30750/#review72067
-----------------------------------------------------------

Ship it!


Ship It!

- Ryan Blue


On Feb. 11, 2015, 3:51 p.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30750/
> -----------------------------------------------------------
> 
> (Updated Feb. 11, 2015, 3:51 p.m.)
> 
> 
> Review request for hive, Ryan Blue, cheng xu, and Dong Chen.
> 
> 
> Bugs: HIVE-9605
>     https://issues.apache.org/jira/browse/HIVE-9605
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Remove wrapper object from parquet nested types (map/array)
> 
> 
> Diffs
> -----
> 
>   itests/hive-jmh/src/main/java/org/apache/hive/benchmark/storage/ColumnarStorageBench.java 61c2eb418e4ad317a603e87240295f9396d1d5b5 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 6621a8768953a9bef54e7a144ae045abcc32f458 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java 4809f9b5882ae409159b422c08c665aa24f796d8 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java fdea782167d63593f6cbde5e7154d771761757f7 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java 62c61fc7502f24e6a032076f384b5a946c1cc9a6 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/DeepParquetHiveMapInspector.java d38c64192e01371c0c98b339113348d2e52cedc3 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveArrayInspector.java 53ca31d0b516c4a941e048e98e7f8f763752c436 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/StandardParquetHiveMapInspector.java 5aa14482899fed5711b40c5554b056d07818afb5 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestArrayCompatibility.java f7f3e57c1607dec3e8dc3a29e819d3aadc65155a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapStructures.java ca4805082fd717d15ed41ca15a730e19da267c8a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetSerDe.java 007a665529857bcec612f638a157aa5043562a15 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestAbstractParquetMapInspector.java ef05150494027ddd70790dcf26b772ebc4cd2b8b 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestDeepParquetHiveMapInspector.java 8646ff4d3413d7d642e2559e1a485d77472b156a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetHiveArrayInspector.java f3a24af2e5f4eeb24e1e286ada19fc9592daacb6 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestStandardParquetHiveMapInspector.java 278419f73b311322dcf3c70abb340bf63d8a4337 
> 
> Diff: https://reviews.apache.org/r/30750/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>


Re: Review Request 30750: HIVE-9605 Remove parquet nested objects from wrapper writable objects

Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30750/
-----------------------------------------------------------

(Updated Feb. 11, 2015, 11:51 p.m.)


Review request for hive, Ryan Blue, cheng xu, and Dong Chen.


Changes
-------

Rellocate imports.


Bugs: HIVE-9605
    https://issues.apache.org/jira/browse/HIVE-9605


Repository: hive-git


Description
-------

Remove wrapper object from parquet nested types (map/array)


Diffs (updated)
-----

  itests/hive-jmh/src/main/java/org/apache/hive/benchmark/storage/ColumnarStorageBench.java 61c2eb418e4ad317a603e87240295f9396d1d5b5 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 6621a8768953a9bef54e7a144ae045abcc32f458 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java 4809f9b5882ae409159b422c08c665aa24f796d8 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java fdea782167d63593f6cbde5e7154d771761757f7 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java 62c61fc7502f24e6a032076f384b5a946c1cc9a6 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/DeepParquetHiveMapInspector.java d38c64192e01371c0c98b339113348d2e52cedc3 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveArrayInspector.java 53ca31d0b516c4a941e048e98e7f8f763752c436 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/StandardParquetHiveMapInspector.java 5aa14482899fed5711b40c5554b056d07818afb5 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestArrayCompatibility.java f7f3e57c1607dec3e8dc3a29e819d3aadc65155a 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapStructures.java ca4805082fd717d15ed41ca15a730e19da267c8a 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetSerDe.java 007a665529857bcec612f638a157aa5043562a15 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestAbstractParquetMapInspector.java ef05150494027ddd70790dcf26b772ebc4cd2b8b 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestDeepParquetHiveMapInspector.java 8646ff4d3413d7d642e2559e1a485d77472b156a 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetHiveArrayInspector.java f3a24af2e5f4eeb24e1e286ada19fc9592daacb6 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestStandardParquetHiveMapInspector.java 278419f73b311322dcf3c70abb340bf63d8a4337 

Diff: https://reviews.apache.org/r/30750/diff/


Testing
-------


Thanks,

Sergio Pena


Re: Review Request 30750: HIVE-9605 Remove parquet nested objects from wrapper writable objects

Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30750/
-----------------------------------------------------------

(Updated Feb. 10, 2015, 9:25 p.m.)


Review request for hive, Ryan Blue, cheng xu, and Dong Chen.


Changes
-------

Fix broken unit-tests


Bugs: HIVE-9605
    https://issues.apache.org/jira/browse/HIVE-9605


Repository: hive-git


Description
-------

Remove wrapper object from parquet nested types (map/array)


Diffs (updated)
-----

  itests/hive-jmh/src/main/java/org/apache/hive/benchmark/storage/ColumnarStorageBench.java 61c2eb418e4ad317a603e87240295f9396d1d5b5 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 6621a8768953a9bef54e7a144ae045abcc32f458 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java 4809f9b5882ae409159b422c08c665aa24f796d8 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java fdea782167d63593f6cbde5e7154d771761757f7 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java 62c61fc7502f24e6a032076f384b5a946c1cc9a6 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/DeepParquetHiveMapInspector.java d38c64192e01371c0c98b339113348d2e52cedc3 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveArrayInspector.java 53ca31d0b516c4a941e048e98e7f8f763752c436 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/StandardParquetHiveMapInspector.java 5aa14482899fed5711b40c5554b056d07818afb5 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestArrayCompatibility.java f7f3e57c1607dec3e8dc3a29e819d3aadc65155a 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapStructures.java ca4805082fd717d15ed41ca15a730e19da267c8a 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetSerDe.java 007a665529857bcec612f638a157aa5043562a15 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestAbstractParquetMapInspector.java ef05150494027ddd70790dcf26b772ebc4cd2b8b 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestDeepParquetHiveMapInspector.java 8646ff4d3413d7d642e2559e1a485d77472b156a 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetHiveArrayInspector.java f3a24af2e5f4eeb24e1e286ada19fc9592daacb6 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestStandardParquetHiveMapInspector.java 278419f73b311322dcf3c70abb340bf63d8a4337 

Diff: https://reviews.apache.org/r/30750/diff/


Testing
-------


Thanks,

Sergio Pena


Re: Review Request 30750: HIVE-9605 Remove parquet nested objects from wrapper writable objects

Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30750/
-----------------------------------------------------------

(Updated Feb. 9, 2015, 4:56 p.m.)


Review request for hive, Ryan Blue, cheng xu, and Dong Chen.


Bugs: HIVE-9605
    https://issues.apache.org/jira/browse/HIVE-9605


Repository: hive-git


Description
-------

Remove wrapper object from parquet nested types (map/array)


Diffs (updated)
-----

  itests/hive-jmh/src/main/java/org/apache/hive/benchmark/storage/ColumnarStorageBench.java d335716be3d286a1b9221dcbd8ccd799f4c6dc66 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 6621a8768953a9bef54e7a144ae045abcc32f458 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java 4809f9b5882ae409159b422c08c665aa24f796d8 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java fdea782167d63593f6cbde5e7154d771761757f7 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java 62c61fc7502f24e6a032076f384b5a946c1cc9a6 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/DeepParquetHiveMapInspector.java d38c64192e01371c0c98b339113348d2e52cedc3 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveArrayInspector.java 53ca31d0b516c4a941e048e98e7f8f763752c436 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/StandardParquetHiveMapInspector.java 5aa14482899fed5711b40c5554b056d07818afb5 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapStructures.java ca4805082fd717d15ed41ca15a730e19da267c8a 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestAbstractParquetMapInspector.java ef05150494027ddd70790dcf26b772ebc4cd2b8b 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestDeepParquetHiveMapInspector.java 8646ff4d3413d7d642e2559e1a485d77472b156a 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetHiveArrayInspector.java f3a24af2e5f4eeb24e1e286ada19fc9592daacb6 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestStandardParquetHiveMapInspector.java 278419f73b311322dcf3c70abb340bf63d8a4337 

Diff: https://reviews.apache.org/r/30750/diff/


Testing
-------


Thanks,

Sergio Pena


Re: Review Request 30750: HIVE-9605 Remove parquet nested objects from wrapper writable objects

Posted by Sergio Pena <se...@cloudera.com>.

> On Feb. 9, 2015, 3:28 a.m., Dong Chen wrote:
> > Looks good! Sergio.
> > 
> > I only have one question. In the 4 inspectors, I think we don't need to check (array length == 0), this may miss the empty map/list case. The original code does this check, but it is for container. What do you think?
> > I left detailed comments for 1 inspector below.

Thanks Dong.

I applied the changes based on your comments.


- Sergio


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


On Feb. 8, 2015, 6:13 a.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30750/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2015, 6:13 a.m.)
> 
> 
> Review request for hive, Ryan Blue, cheng xu, and Dong Chen.
> 
> 
> Bugs: HIVE-9605
>     https://issues.apache.org/jira/browse/HIVE-9605
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Remove wrapper object from parquet nested types (map/array)
> 
> 
> Diffs
> -----
> 
>   itests/hive-jmh/src/main/java/org/apache/hive/benchmark/storage/ColumnarStorageBench.java d335716be3d286a1b9221dcbd8ccd799f4c6dc66 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 6621a8768953a9bef54e7a144ae045abcc32f458 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java 4809f9b5882ae409159b422c08c665aa24f796d8 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java fdea782167d63593f6cbde5e7154d771761757f7 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java 62c61fc7502f24e6a032076f384b5a946c1cc9a6 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/DeepParquetHiveMapInspector.java d38c64192e01371c0c98b339113348d2e52cedc3 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveArrayInspector.java 53ca31d0b516c4a941e048e98e7f8f763752c436 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/StandardParquetHiveMapInspector.java 5aa14482899fed5711b40c5554b056d07818afb5 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapStructures.java ca4805082fd717d15ed41ca15a730e19da267c8a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestAbstractParquetMapInspector.java ef05150494027ddd70790dcf26b772ebc4cd2b8b 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestDeepParquetHiveMapInspector.java 8646ff4d3413d7d642e2559e1a485d77472b156a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetHiveArrayInspector.java f3a24af2e5f4eeb24e1e286ada19fc9592daacb6 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestStandardParquetHiveMapInspector.java 278419f73b311322dcf3c70abb340bf63d8a4337 
> 
> Diff: https://reviews.apache.org/r/30750/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>


Re: Review Request 30750: HIVE-9605 Remove parquet nested objects from wrapper writable objects

Posted by Dong Chen <do...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30750/#review71603
-----------------------------------------------------------


Looks good! Sergio.

I only have one question. In the 4 inspectors, I think we don't need to check (array length == 0), this may miss the empty map/list case. The original code does this check, but it is for container. What do you think?
I left detailed comments for 1 inspector below.


ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java
<https://reviews.apache.org/r/30750/#comment117426>

    In original wrapper, it has to check null and length to ensure there is an ArrayWritable of Map. Then get it.
    
    As we removed the wrapper, and get the mapArray already, maybe we just need to check null here. If length is 0, I think we should return an empty map instead of null.



ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java
<https://reviews.apache.org/r/30750/#comment117427>

    how about just check null? 
    Length being 0 might be a normal case.


- Dong Chen


On Feb. 8, 2015, 6:13 a.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30750/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2015, 6:13 a.m.)
> 
> 
> Review request for hive, Ryan Blue, cheng xu, and Dong Chen.
> 
> 
> Bugs: HIVE-9605
>     https://issues.apache.org/jira/browse/HIVE-9605
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Remove wrapper object from parquet nested types (map/array)
> 
> 
> Diffs
> -----
> 
>   itests/hive-jmh/src/main/java/org/apache/hive/benchmark/storage/ColumnarStorageBench.java d335716be3d286a1b9221dcbd8ccd799f4c6dc66 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 6621a8768953a9bef54e7a144ae045abcc32f458 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java 4809f9b5882ae409159b422c08c665aa24f796d8 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java fdea782167d63593f6cbde5e7154d771761757f7 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java 62c61fc7502f24e6a032076f384b5a946c1cc9a6 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/DeepParquetHiveMapInspector.java d38c64192e01371c0c98b339113348d2e52cedc3 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveArrayInspector.java 53ca31d0b516c4a941e048e98e7f8f763752c436 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/StandardParquetHiveMapInspector.java 5aa14482899fed5711b40c5554b056d07818afb5 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapStructures.java ca4805082fd717d15ed41ca15a730e19da267c8a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestAbstractParquetMapInspector.java ef05150494027ddd70790dcf26b772ebc4cd2b8b 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestDeepParquetHiveMapInspector.java 8646ff4d3413d7d642e2559e1a485d77472b156a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetHiveArrayInspector.java f3a24af2e5f4eeb24e1e286ada19fc9592daacb6 
>   ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestStandardParquetHiveMapInspector.java 278419f73b311322dcf3c70abb340bf63d8a4337 
> 
> Diff: https://reviews.apache.org/r/30750/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>


Re: Review Request 30750: HIVE-9605 Remove parquet nested objects from wrapper writable objects

Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30750/
-----------------------------------------------------------

(Updated Feb. 8, 2015, 6:13 a.m.)


Review request for hive, Ryan Blue, cheng xu, and Dong Chen.


Changes
-------

Removed wraplist() method as it is never used anymore.


Bugs: HIVE-9605
    https://issues.apache.org/jira/browse/HIVE-9605


Repository: hive-git


Description
-------

Remove wrapper object from parquet nested types (map/array)


Diffs (updated)
-----

  itests/hive-jmh/src/main/java/org/apache/hive/benchmark/storage/ColumnarStorageBench.java d335716be3d286a1b9221dcbd8ccd799f4c6dc66 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 6621a8768953a9bef54e7a144ae045abcc32f458 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java 4809f9b5882ae409159b422c08c665aa24f796d8 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java fdea782167d63593f6cbde5e7154d771761757f7 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/AbstractParquetMapInspector.java 62c61fc7502f24e6a032076f384b5a946c1cc9a6 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/DeepParquetHiveMapInspector.java d38c64192e01371c0c98b339113348d2e52cedc3 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveArrayInspector.java 53ca31d0b516c4a941e048e98e7f8f763752c436 
  ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/StandardParquetHiveMapInspector.java 5aa14482899fed5711b40c5554b056d07818afb5 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestMapStructures.java ca4805082fd717d15ed41ca15a730e19da267c8a 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestAbstractParquetMapInspector.java ef05150494027ddd70790dcf26b772ebc4cd2b8b 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestDeepParquetHiveMapInspector.java 8646ff4d3413d7d642e2559e1a485d77472b156a 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetHiveArrayInspector.java f3a24af2e5f4eeb24e1e286ada19fc9592daacb6 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestStandardParquetHiveMapInspector.java 278419f73b311322dcf3c70abb340bf63d8a4337 

Diff: https://reviews.apache.org/r/30750/diff/


Testing
-------


Thanks,

Sergio Pena