You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Ashish Singh <as...@cloudera.com> on 2015/04/17 05:01:16 UTC
Re: Review Request 28372: Address review comments
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28372/
-----------------------------------------------------------
(Updated April 17, 2015, 3:01 a.m.)
Review request for hive.
Changes
-------
Removed parquet.file table property, restricting the capability of automatic schema generation to external tables.
Summary (updated)
-----------------
Address review comments
Bugs: HIVE-8950
https://issues.apache.org/jira/browse/HIVE-8950
Repository: hive-git
Description
-------
HIVE-8950: Add support in ParquetHiveSerde to create table schema from a parquet file
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e138800e6dadd6fe76345f21eb76c906165c438d
hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java 53d913435cd43c96f044cf8668461fc686817ef4
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 3f267ff0eb20560c36a19b74353f9d6749c8b333
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 907ad2b8d7bae9b13eb4d9605ff2a3fe60c03ee8
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ParquetSchemaReader.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ParquetToHiveSchemaConverter.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java 7fd5e9612d4e3c9bf3b816bc48dbdbe59fb8a5a8
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java a029f10c116fd46070b2d41790043f0a7001390f
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetToHiveSchemaConverter.java PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_multi_field_struct_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_optional_elements_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_required_elements_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_single_field_struct_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_structs_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_structs_gen_schema_ext.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_unannotated_groups_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_unannotated_primitives_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_avro_array_of_primitives_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_avro_array_of_single_field_struct_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_decimal_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_thrift_array_of_primitives_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_thrift_array_of_single_field_struct_gen_schema.q PRE-CREATION
ql/src/test/results/clientpositive/create_view_partitioned.q.out ebf9a6bc4f2321d7f539b7a445b3f279e3285b8a
ql/src/test/results/clientpositive/parquet_array_of_multi_field_struct_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_array_of_optional_elements_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_array_of_required_elements_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_array_of_single_field_struct_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_array_of_structs_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_array_of_structs_gen_schema_ext.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_array_of_unannotated_groups_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_array_of_unannotated_primitives_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_avro_array_of_primitives_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_avro_array_of_single_field_struct_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_decimal_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_thrift_array_of_primitives_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_thrift_array_of_single_field_struct_gen_schema.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/28372/diff/
Testing
-------
Tested by adding appropriate qTests.
Thanks,
Ashish Singh
Re: Review Request 28372: HIVE-8950: Add support in ParquetHiveSerde
to create table schema from a parquet file
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28372/
-----------------------------------------------------------
(Updated April 30, 2015, 12:13 a.m.)
Review request for hive.
Changes
-------
Addressed Ryan's review comments.
Summary (updated)
-----------------
HIVE-8950: Add support in ParquetHiveSerde to create table schema from a parquet file
Bugs: HIVE-8950
https://issues.apache.org/jira/browse/HIVE-8950
Repository: hive-git
Description
-------
HIVE-8950: Add support in ParquetHiveSerde to create table schema from a parquet file
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 65ec1b94f081379d8c6abf0503b6c50305a35ef4
hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java 50e022da57575cc3020de34a0d34510f224a37de
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 3f267ff0eb20560c36a19b74353f9d6749c8b333
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java b89b07a7460e456918f7cb8054f8f2ff1a752033
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ParquetSchemaReader.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ParquetToHiveSchemaConverter.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java 7fd5e9612d4e3c9bf3b816bc48dbdbe59fb8a5a8
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 39c48b52665e4de024cf2abc99e0637549867fa3
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetToHiveSchemaConverter.java PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_multi_field_struct_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_optional_elements_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_required_elements_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_single_field_struct_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_structs_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_structs_gen_schema_ext.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_unannotated_groups_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_array_of_unannotated_primitives_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_avro_array_of_primitives_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_avro_array_of_single_field_struct_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_decimal_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_thrift_array_of_primitives_gen_schema.q PRE-CREATION
ql/src/test/queries/clientpositive/parquet_thrift_array_of_single_field_struct_gen_schema.q PRE-CREATION
ql/src/test/results/clientpositive/create_view_partitioned.q.out ebf9a6bc4f2321d7f539b7a445b3f279e3285b8a
ql/src/test/results/clientpositive/parquet_array_of_multi_field_struct_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_array_of_optional_elements_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_array_of_required_elements_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_array_of_single_field_struct_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_array_of_structs_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_array_of_structs_gen_schema_ext.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_array_of_unannotated_groups_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_array_of_unannotated_primitives_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_avro_array_of_primitives_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_avro_array_of_single_field_struct_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_decimal_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_thrift_array_of_primitives_gen_schema.q.out PRE-CREATION
ql/src/test/results/clientpositive/parquet_thrift_array_of_single_field_struct_gen_schema.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/28372/diff/
Testing
-------
Tested by adding appropriate qTests.
Thanks,
Ashish Singh
Re: Review Request 28372: Address review comments
Posted by Ashish Singh <as...@cloudera.com>.
> On April 28, 2015, 9:30 p.m., Ryan Blue wrote:
> > Looking good to me. I found a few things to fix, and I'm also wondering where the qtest outputs are.
Thanks for the review Ryan!
qtest outputs are in *.q.out files.
- Ashish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28372/#review81884
-----------------------------------------------------------
On April 17, 2015, 3:01 a.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28372/
> -----------------------------------------------------------
>
> (Updated April 17, 2015, 3:01 a.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-8950
> https://issues.apache.org/jira/browse/HIVE-8950
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-8950: Add support in ParquetHiveSerde to create table schema from a parquet file
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e138800e6dadd6fe76345f21eb76c906165c438d
> hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java 53d913435cd43c96f044cf8668461fc686817ef4
> metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 3f267ff0eb20560c36a19b74353f9d6749c8b333
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 907ad2b8d7bae9b13eb4d9605ff2a3fe60c03ee8
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ParquetSchemaReader.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ParquetToHiveSchemaConverter.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java 7fd5e9612d4e3c9bf3b816bc48dbdbe59fb8a5a8
> ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java a029f10c116fd46070b2d41790043f0a7001390f
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetToHiveSchemaConverter.java PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_multi_field_struct_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_optional_elements_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_required_elements_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_single_field_struct_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_structs_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_structs_gen_schema_ext.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_unannotated_groups_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_unannotated_primitives_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_avro_array_of_primitives_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_avro_array_of_single_field_struct_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_decimal_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_thrift_array_of_primitives_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_thrift_array_of_single_field_struct_gen_schema.q PRE-CREATION
> ql/src/test/results/clientpositive/create_view_partitioned.q.out ebf9a6bc4f2321d7f539b7a445b3f279e3285b8a
> ql/src/test/results/clientpositive/parquet_array_of_multi_field_struct_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_array_of_optional_elements_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_array_of_required_elements_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_array_of_single_field_struct_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_array_of_structs_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_array_of_structs_gen_schema_ext.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_array_of_unannotated_groups_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_array_of_unannotated_primitives_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_avro_array_of_primitives_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_avro_array_of_single_field_struct_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_decimal_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_thrift_array_of_primitives_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_thrift_array_of_single_field_struct_gen_schema.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/28372/diff/
>
>
> Testing
> -------
>
> Tested by adding appropriate qTests.
>
>
> Thanks,
>
> Ashish Singh
>
>
Re: Review Request 28372: Address review comments
Posted by Ryan Blue <bl...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28372/#review81884
-----------------------------------------------------------
Looking good to me. I found a few things to fix, and I'm also wondering where the qtest outputs are.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ParquetToHiveSchemaConverter.java
<https://reviews.apache.org/r/28372/#comment132397>
Sergio discovered that MAP_KEY_VALUE was incorrectly used in place of MAP. So instead of throwing UnsupportedOperationException, MAP_KEY_VALUE should be used as a synonym for MAP. That way we can read old data.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java
<https://reviews.apache.org/r/28372/#comment132398>
Shouldn't you also pass the exception so that the stack trace is printed?
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetToHiveSchemaConverter.java
<https://reviews.apache.org/r/28372/#comment132399>
Minor: Would be good to use the Types API instead of constructors.
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetToHiveSchemaConverter.java
<https://reviews.apache.org/r/28372/#comment132401>
I think the maps and lists should be constructed using the current best practice, ConversionPatterns, instead of by hand. That way we're testing what is actually going to be in data files.
- Ryan Blue
On April 16, 2015, 8:01 p.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28372/
> -----------------------------------------------------------
>
> (Updated April 16, 2015, 8:01 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-8950
> https://issues.apache.org/jira/browse/HIVE-8950
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-8950: Add support in ParquetHiveSerde to create table schema from a parquet file
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e138800e6dadd6fe76345f21eb76c906165c438d
> hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java 53d913435cd43c96f044cf8668461fc686817ef4
> metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 3f267ff0eb20560c36a19b74353f9d6749c8b333
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 907ad2b8d7bae9b13eb4d9605ff2a3fe60c03ee8
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ParquetSchemaReader.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ParquetToHiveSchemaConverter.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java 7fd5e9612d4e3c9bf3b816bc48dbdbe59fb8a5a8
> ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java a029f10c116fd46070b2d41790043f0a7001390f
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetToHiveSchemaConverter.java PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_multi_field_struct_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_optional_elements_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_required_elements_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_single_field_struct_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_structs_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_structs_gen_schema_ext.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_unannotated_groups_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_array_of_unannotated_primitives_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_avro_array_of_primitives_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_avro_array_of_single_field_struct_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_decimal_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_thrift_array_of_primitives_gen_schema.q PRE-CREATION
> ql/src/test/queries/clientpositive/parquet_thrift_array_of_single_field_struct_gen_schema.q PRE-CREATION
> ql/src/test/results/clientpositive/create_view_partitioned.q.out ebf9a6bc4f2321d7f539b7a445b3f279e3285b8a
> ql/src/test/results/clientpositive/parquet_array_of_multi_field_struct_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_array_of_optional_elements_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_array_of_required_elements_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_array_of_single_field_struct_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_array_of_structs_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_array_of_structs_gen_schema_ext.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_array_of_unannotated_groups_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_array_of_unannotated_primitives_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_avro_array_of_primitives_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_avro_array_of_single_field_struct_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_decimal_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_thrift_array_of_primitives_gen_schema.q.out PRE-CREATION
> ql/src/test/results/clientpositive/parquet_thrift_array_of_single_field_struct_gen_schema.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/28372/diff/
>
>
> Testing
> -------
>
> Tested by adding appropriate qTests.
>
>
> Thanks,
>
> Ashish Singh
>
>