You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Szehon Ho <sz...@cloudera.com> on 2015/01/28 00:07:30 UTC
Review Request 30337: HIVE-9482 : Hive parquet timestamp compatibility
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30337/
-----------------------------------------------------------
Review request for hive and Brock Noland.
Repository: hive-git
Description
-------
In current Hive implementation, timestamps are stored in UTC (converted from current timezone), based on original parquet timestamp spec.
However, we find this is not compatibility with other tools, and after some investigation it is not the way of the other file formats, or even some databases (Hive Timestamp is more equivalent of 'timestamp without timezone' datatype).
This is the first part of the fix, which will restore compatibility with parquet-timestamp files generated by external tools by skipping conversion on reading.
Later fix will change the write path to not convert, and stop the read-conversion even for files written by Hive itself.
Diffs
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 64e7e0a
data/files/parquet_external_time.parq PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ConverterParent.java a86d6f4
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/DataWritableRecordConverter.java 000e8ea
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java 23bb364
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 872900b
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java 11772be
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveStructConverter.java eeb3838
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java af28b4c
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java 3f8e4d7
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java 4e4d7fd
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java c647b24
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java 41b5f1c
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetTimestampUtils.java 2e788bd
ql/src/test/queries/clientpositive/parquet_external_time.q PRE-CREATION
ql/src/test/results/clientpositive/parquet_external_time.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/30337/diff/
Testing
-------
Added new unit tests (TestParquetTimestampUtils) to test non-conversion code-path.
Also added new q-test, to read a parquet timestamp-file generated by an external tool, in this case Impala.
Thanks,
Szehon Ho
Re: Review Request 30337: HIVE-9482 : Hive parquet timestamp
compatibility
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30337/#review70096
-----------------------------------------------------------
Ship it!
Ship It!
- Brock Noland
On Jan. 28, 2015, 8:10 p.m., Szehon Ho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30337/
> -----------------------------------------------------------
>
> (Updated Jan. 28, 2015, 8:10 p.m.)
>
>
> Review request for hive and Brock Noland.
>
>
> Bugs: HIVE-9482
> https://issues.apache.org/jira/browse/HIVE-9482
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> In current Hive implementation, timestamps are stored in UTC (converted from current timezone), based on original parquet timestamp spec.
> However, we find this is not compatibility with other tools, and after some investigation it is not the way of the other file formats, or even some databases (Hive Timestamp is more equivalent of 'timestamp without timezone' datatype).
>
> This is the first part of the fix, which will restore compatibility with parquet-timestamp files generated by external tools by skipping conversion on reading.
>
> Later fix will change the write path to not convert, and stop the read-conversion even for files written by Hive itself.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 64e7e0a
> data/files/parquet_external_time.parq PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ConverterParent.java a86d6f4
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/DataWritableRecordConverter.java 000e8ea
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java 23bb364
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 872900b
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java 11772be
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveStructConverter.java eeb3838
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java af28b4c
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java 3f8e4d7
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java 4e4d7fd
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java c647b24
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java 41b5f1c
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetTimestampUtils.java 2e788bd
> ql/src/test/queries/clientpositive/parquet_external_time.q PRE-CREATION
> ql/src/test/results/clientpositive/parquet_external_time.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/30337/diff/
>
>
> Testing
> -------
>
> Added new unit tests (TestParquetTimestampUtils) to test non-conversion code-path.
>
> Also added new q-test, to read a parquet timestamp-file generated by an external tool, in this case Impala.
>
>
> Thanks,
>
> Szehon Ho
>
>
Re: Review Request 30337: HIVE-9482 : Hive parquet timestamp
compatibility
Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30337/
-----------------------------------------------------------
(Updated Jan. 28, 2015, 8:10 p.m.)
Review request for hive and Brock Noland.
Changes
-------
Address review comments.
Bugs: HIVE-9482
https://issues.apache.org/jira/browse/HIVE-9482
Repository: hive-git
Description
-------
In current Hive implementation, timestamps are stored in UTC (converted from current timezone), based on original parquet timestamp spec.
However, we find this is not compatibility with other tools, and after some investigation it is not the way of the other file formats, or even some databases (Hive Timestamp is more equivalent of 'timestamp without timezone' datatype).
This is the first part of the fix, which will restore compatibility with parquet-timestamp files generated by external tools by skipping conversion on reading.
Later fix will change the write path to not convert, and stop the read-conversion even for files written by Hive itself.
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 64e7e0a
data/files/parquet_external_time.parq PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ConverterParent.java a86d6f4
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/DataWritableRecordConverter.java 000e8ea
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java 23bb364
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 872900b
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java 11772be
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveStructConverter.java eeb3838
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java af28b4c
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java 3f8e4d7
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java 4e4d7fd
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java c647b24
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java 41b5f1c
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetTimestampUtils.java 2e788bd
ql/src/test/queries/clientpositive/parquet_external_time.q PRE-CREATION
ql/src/test/results/clientpositive/parquet_external_time.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/30337/diff/
Testing
-------
Added new unit tests (TestParquetTimestampUtils) to test non-conversion code-path.
Also added new q-test, to read a parquet timestamp-file generated by an external tool, in this case Impala.
Thanks,
Szehon Ho
Re: Review Request 30337: HIVE-9482 : Hive parquet timestamp
compatibility
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30337/#review70050
-----------------------------------------------------------
Looks great! Two minor items below.
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java
<https://reviews.apache.org/r/30337/#comment114993>
is "createdBy" optional? I am wondering if should wrap the get call in Strings.nullToEmpty?
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java
<https://reviews.apache.org/r/30337/#comment114995>
Since this is JobConf will this always defined? Perhaps we should use the static HiveConf. getBoolVar?
- Brock Noland
On Jan. 27, 2015, 11:07 p.m., Szehon Ho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30337/
> -----------------------------------------------------------
>
> (Updated Jan. 27, 2015, 11:07 p.m.)
>
>
> Review request for hive and Brock Noland.
>
>
> Bugs: HIVE-9482
> https://issues.apache.org/jira/browse/HIVE-9482
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> In current Hive implementation, timestamps are stored in UTC (converted from current timezone), based on original parquet timestamp spec.
> However, we find this is not compatibility with other tools, and after some investigation it is not the way of the other file formats, or even some databases (Hive Timestamp is more equivalent of 'timestamp without timezone' datatype).
>
> This is the first part of the fix, which will restore compatibility with parquet-timestamp files generated by external tools by skipping conversion on reading.
>
> Later fix will change the write path to not convert, and stop the read-conversion even for files written by Hive itself.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 64e7e0a
> data/files/parquet_external_time.parq PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ConverterParent.java a86d6f4
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/DataWritableRecordConverter.java 000e8ea
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java 23bb364
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 872900b
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java 11772be
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveStructConverter.java eeb3838
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java af28b4c
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java 3f8e4d7
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java 4e4d7fd
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java c647b24
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java 41b5f1c
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetTimestampUtils.java 2e788bd
> ql/src/test/queries/clientpositive/parquet_external_time.q PRE-CREATION
> ql/src/test/results/clientpositive/parquet_external_time.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/30337/diff/
>
>
> Testing
> -------
>
> Added new unit tests (TestParquetTimestampUtils) to test non-conversion code-path.
>
> Also added new q-test, to read a parquet timestamp-file generated by an external tool, in this case Impala.
>
>
> Thanks,
>
> Szehon Ho
>
>
Re: Review Request 30337: HIVE-9482 : Hive parquet timestamp
compatibility
Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30337/
-----------------------------------------------------------
(Updated Jan. 27, 2015, 11:07 p.m.)
Review request for hive and Brock Noland.
Bugs: HIVE-9482
https://issues.apache.org/jira/browse/HIVE-9482
Repository: hive-git
Description
-------
In current Hive implementation, timestamps are stored in UTC (converted from current timezone), based on original parquet timestamp spec.
However, we find this is not compatibility with other tools, and after some investigation it is not the way of the other file formats, or even some databases (Hive Timestamp is more equivalent of 'timestamp without timezone' datatype).
This is the first part of the fix, which will restore compatibility with parquet-timestamp files generated by external tools by skipping conversion on reading.
Later fix will change the write path to not convert, and stop the read-conversion even for files written by Hive itself.
Diffs
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 64e7e0a
data/files/parquet_external_time.parq PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ConverterParent.java a86d6f4
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/DataWritableRecordConverter.java 000e8ea
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java 23bb364
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveCollectionConverter.java 872900b
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveGroupConverter.java 11772be
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveStructConverter.java eeb3838
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/Repeated.java af28b4c
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java 3f8e4d7
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java 4e4d7fd
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java c647b24
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java 41b5f1c
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetTimestampUtils.java 2e788bd
ql/src/test/queries/clientpositive/parquet_external_time.q PRE-CREATION
ql/src/test/results/clientpositive/parquet_external_time.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/30337/diff/
Testing
-------
Added new unit tests (TestParquetTimestampUtils) to test non-conversion code-path.
Also added new q-test, to read a parquet timestamp-file generated by an external tool, in this case Impala.
Thanks,
Szehon Ho