You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Chaoyu Tang <ct...@gmail.com> on 2015/11/29 18:47:51 UTC
Review Request 40785: HIVE-12506: SHOW CREATE TABLE command creates a
table that does not work for RCFile format
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40785/
-----------------------------------------------------------
Review request for hive.
Repository: hive-git
Description
-------
More than one SerDe in Hive (e.g. LazySimpleSerDe, ColumnarSerDe) use the DELIMITED properties, so "show create table" output with DELIMTED instead of SERDE [WITH SERDEPROPERTIES] is not sufficient to be used to recreate the same table.
Diffs
-----
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 02cbd76
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java a210b95
ql/src/test/results/clientpositive/nullformat.q.out 6cfc2b8
ql/src/test/results/clientpositive/nullformatCTAS.q.out 7adca68
ql/src/test/results/clientpositive/show_create_table_delimited.q.out 79f29de
Diff: https://reviews.apache.org/r/40785/diff/
Testing
-------
1. Manual tests passed
2. has been submitted for pre-commit build.
Thanks,
Chaoyu Tang
Re: Review Request 40785: HIVE-12506: SHOW CREATE TABLE command
creates a table that does not work for RCFile format
Posted by Jimmy Xiang <jx...@apache.org>.
> On Dec. 1, 2015, 10:45 p.m., Jimmy Xiang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java, line 2128
> > <https://reviews.apache.org/r/40785/diff/1/?file=1148109#file1148109line2128>
> >
> > If this is the only statement in the block, can we merge two if statements into one?
>
> Chaoyu Tang wrote:
> Jimmy, could you clarify? this is the code with patch:
> ---
> if (tbl.getStorageHandler() == null) {
> // If serialization.format property has the default value, it will not to be included in
> // SERDE properties
> if (MetaStoreUtils.DEFAULT_SERIALIZATION_FORMAT.equals(serdeParams.get(
> serdeConstants.SERIALIZATION_FORMAT))){
> serdeParams.remove(serdeConstants.SERIALIZATION_FORMAT);
> }
> if (!serdeParams.isEmpty()) {
> appendSerdeParams(tbl_row_format, serdeParams).append(" \n");
> }
> tbl_row_format.append("STORED AS INPUTFORMAT \n '" +
> escapeHiveCommand(sd.getInputFormat()) + "' \n");
> tbl_row_format.append("OUTPUTFORMAT \n '" +
> escapeHiveCommand(sd.getOutputFormat()) + "'");
> } else {
> ...
> ---
> You meant which two if can be consolidated?
I see. There is a tab so I thought there is just one if statement in the block by mistake.
> On Dec. 1, 2015, 10:45 p.m., Jimmy Xiang wrote:
> > ql/src/test/results/clientpositive/show_create_table_delimited.q.out, line 36
> > <https://reviews.apache.org/r/40785/diff/1/?file=1148112#file1148112line36>
> >
> > Now we expose those serde properties names. This is ok, right?
>
> Chaoyu Tang wrote:
> I think it should be fine since other Hive commands like desc extended tblName already expose these serde property names in their output. for example, here is partial output of desc extended tblName:
> serdeInfo:SerDeInfo(name:null, serializationLib:org.apache.hadoop.hive.serde2.columnar.ColumnarSerDe, parameters:{colelction.delim=,, mapkey.delim=:, serialization.format=|, field.delim=|}),
ok
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40785/#review108573
-----------------------------------------------------------
On Nov. 29, 2015, 5:47 p.m., Chaoyu Tang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40785/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2015, 5:47 p.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> More than one SerDe in Hive (e.g. LazySimpleSerDe, ColumnarSerDe) use the DELIMITED properties, so "show create table" output with DELIMTED instead of SERDE [WITH SERDEPROPERTIES] is not sufficient to be used to recreate the same table.
>
>
> Diffs
> -----
>
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 02cbd76
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java a210b95
> ql/src/test/results/clientpositive/nullformat.q.out 6cfc2b8
> ql/src/test/results/clientpositive/nullformatCTAS.q.out 7adca68
> ql/src/test/results/clientpositive/show_create_table_delimited.q.out 79f29de
>
> Diff: https://reviews.apache.org/r/40785/diff/
>
>
> Testing
> -------
>
> 1. Manual tests passed
> 2. has been submitted for pre-commit build.
>
>
> Thanks,
>
> Chaoyu Tang
>
>
Re: Review Request 40785: HIVE-12506: SHOW CREATE TABLE command
creates a table that does not work for RCFile format
Posted by Chaoyu Tang <ct...@gmail.com>.
> On Dec. 1, 2015, 10:45 p.m., Jimmy Xiang wrote:
> > ql/src/test/results/clientpositive/show_create_table_delimited.q.out, line 36
> > <https://reviews.apache.org/r/40785/diff/1/?file=1148112#file1148112line36>
> >
> > Now we expose those serde properties names. This is ok, right?
I think it should be fine since other Hive commands like desc extended tblName already expose these serde property names in their output. for example, here is partial output of desc extended tblName:
serdeInfo:SerDeInfo(name:null, serializationLib:org.apache.hadoop.hive.serde2.columnar.ColumnarSerDe, parameters:{colelction.delim=,, mapkey.delim=:, serialization.format=|, field.delim=|}),
> On Dec. 1, 2015, 10:45 p.m., Jimmy Xiang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java, line 2124
> > <https://reviews.apache.org/r/40785/diff/1/?file=1148109#file1148109line2124>
> >
> > So now we always show these params? Do we have some default value for each paramter and skip those properties with the default value?
No, Usually if a serde paremeter (e.g. line.delimiter, field.delimiter) is not explicitly specified in the table creation DDL, it is not populated to HMS database during table creation, therefore it won't be shown in the "show create table" output. The property serialization.format is the only exception, so it is removed from the output if it has default value.
> On Dec. 1, 2015, 10:45 p.m., Jimmy Xiang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java, line 2128
> > <https://reviews.apache.org/r/40785/diff/1/?file=1148109#file1148109line2128>
> >
> > If this is the only statement in the block, can we merge two if statements into one?
Jimmy, could you clarify? this is the code with patch:
---
if (tbl.getStorageHandler() == null) {
// If serialization.format property has the default value, it will not to be included in
// SERDE properties
if (MetaStoreUtils.DEFAULT_SERIALIZATION_FORMAT.equals(serdeParams.get(
serdeConstants.SERIALIZATION_FORMAT))){
serdeParams.remove(serdeConstants.SERIALIZATION_FORMAT);
}
if (!serdeParams.isEmpty()) {
appendSerdeParams(tbl_row_format, serdeParams).append(" \n");
}
tbl_row_format.append("STORED AS INPUTFORMAT \n '" +
escapeHiveCommand(sd.getInputFormat()) + "' \n");
tbl_row_format.append("OUTPUTFORMAT \n '" +
escapeHiveCommand(sd.getOutputFormat()) + "'");
} else {
...
---
You meant which two if can be consolidated?
- Chaoyu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40785/#review108573
-----------------------------------------------------------
On Nov. 29, 2015, 5:47 p.m., Chaoyu Tang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40785/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2015, 5:47 p.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> More than one SerDe in Hive (e.g. LazySimpleSerDe, ColumnarSerDe) use the DELIMITED properties, so "show create table" output with DELIMTED instead of SERDE [WITH SERDEPROPERTIES] is not sufficient to be used to recreate the same table.
>
>
> Diffs
> -----
>
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 02cbd76
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java a210b95
> ql/src/test/results/clientpositive/nullformat.q.out 6cfc2b8
> ql/src/test/results/clientpositive/nullformatCTAS.q.out 7adca68
> ql/src/test/results/clientpositive/show_create_table_delimited.q.out 79f29de
>
> Diff: https://reviews.apache.org/r/40785/diff/
>
>
> Testing
> -------
>
> 1. Manual tests passed
> 2. has been submitted for pre-commit build.
>
>
> Thanks,
>
> Chaoyu Tang
>
>
Re: Review Request 40785: HIVE-12506: SHOW CREATE TABLE command
creates a table that does not work for RCFile format
Posted by Jimmy Xiang <jx...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40785/#review108573
-----------------------------------------------------------
Looks ok to me. Just some questions.
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java (line 2122)
<https://reviews.apache.org/r/40785/#comment168024>
So now we always show these params? Do we have some default value for each paramter and skip those properties with the default value?
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java (line 2126)
<https://reviews.apache.org/r/40785/#comment168023>
If this is the only statement in the block, can we merge two if statements into one?
ql/src/test/results/clientpositive/show_create_table_delimited.q.out (line 36)
<https://reviews.apache.org/r/40785/#comment168025>
Now we expose those serde properties names. This is ok, right?
- Jimmy Xiang
On Nov. 29, 2015, 5:47 p.m., Chaoyu Tang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40785/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2015, 5:47 p.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> More than one SerDe in Hive (e.g. LazySimpleSerDe, ColumnarSerDe) use the DELIMITED properties, so "show create table" output with DELIMTED instead of SERDE [WITH SERDEPROPERTIES] is not sufficient to be used to recreate the same table.
>
>
> Diffs
> -----
>
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 02cbd76
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java a210b95
> ql/src/test/results/clientpositive/nullformat.q.out 6cfc2b8
> ql/src/test/results/clientpositive/nullformatCTAS.q.out 7adca68
> ql/src/test/results/clientpositive/show_create_table_delimited.q.out 79f29de
>
> Diff: https://reviews.apache.org/r/40785/diff/
>
>
> Testing
> -------
>
> 1. Manual tests passed
> 2. has been submitted for pre-commit build.
>
>
> Thanks,
>
> Chaoyu Tang
>
>
Re: Review Request 40785: HIVE-12506: SHOW CREATE TABLE command
creates a table that does not work for RCFile format
Posted by Jimmy Xiang <jx...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40785/#review108582
-----------------------------------------------------------
Ship it!
Ship It!
- Jimmy Xiang
On Nov. 29, 2015, 5:47 p.m., Chaoyu Tang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40785/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2015, 5:47 p.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> More than one SerDe in Hive (e.g. LazySimpleSerDe, ColumnarSerDe) use the DELIMITED properties, so "show create table" output with DELIMTED instead of SERDE [WITH SERDEPROPERTIES] is not sufficient to be used to recreate the same table.
>
>
> Diffs
> -----
>
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 02cbd76
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java a210b95
> ql/src/test/results/clientpositive/nullformat.q.out 6cfc2b8
> ql/src/test/results/clientpositive/nullformatCTAS.q.out 7adca68
> ql/src/test/results/clientpositive/show_create_table_delimited.q.out 79f29de
>
> Diff: https://reviews.apache.org/r/40785/diff/
>
>
> Testing
> -------
>
> 1. Manual tests passed
> 2. has been submitted for pre-commit build.
>
>
> Thanks,
>
> Chaoyu Tang
>
>