You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Peter Vary <pv...@cloudera.com> on 2016/07/11 09:36:57 UTC
Review Request 49882: HIVE-14146: Column comments with "\n" character
"corrupts" table metadata
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49882/
-----------------------------------------------------------
Review request for hive, Aihua Xu, Sergio Pena, Szehon Ho, and Vihang Karajgaonkar.
Bugs: HIVE-14146
https://issues.apache.org/jira/browse/HIVE-14146
Repository: hive-git
Description
-------
The patch contains:
- MetaDataFormatUtils changes - to escape the \n in index, and column comments (table comments are already handled)
- TextMetaDataFormatter changes - to escape the \n in database comments
- DDLTask chages - to escape \n in show create table result
- New query test, to test the escaping
Diffs
-----
common/src/java/org/apache/hive/common/util/HiveStringUtils.java 72c3fa9
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java bb43950
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java 03803bb
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java 47d67b1
ql/src/test/queries/clientpositive/escape_comments.q PRE-CREATION
ql/src/test/results/clientpositive/escape_comments.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/49882/diff/
Testing
-------
New unit test and manually
Thanks,
Peter Vary
Re: Review Request 49882: HIVE-14146: Column comments with "\n"
character "corrupts" table metadata
Posted by Aihua Xu <ax...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49882/#review141686
-----------------------------------------------------------
common/src/java/org/apache/hive/common/util/HiveStringUtils.java (line 644)
<https://reviews.apache.org/r/49882/#comment207053>
Somehow I feel we should have a better name for this function since we are escaping single quote and \ as well.
Maybe just the orignal one escapeHiveCommand()?
- Aihua Xu
On July 11, 2016, 9:36 a.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49882/
> -----------------------------------------------------------
>
> (Updated July 11, 2016, 9:36 a.m.)
>
>
> Review request for hive, Aihua Xu, Sergio Pena, Szehon Ho, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-14146
> https://issues.apache.org/jira/browse/HIVE-14146
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> The patch contains:
> - MetaDataFormatUtils changes - to escape the \n in index, and column comments (table comments are already handled)
> - TextMetaDataFormatter changes - to escape the \n in database comments
> - DDLTask chages - to escape \n in show create table result
> - New query test, to test the escaping
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hive/common/util/HiveStringUtils.java 72c3fa9
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java bb43950
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java 03803bb
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java 47d67b1
> ql/src/test/queries/clientpositive/escape_comments.q PRE-CREATION
> ql/src/test/results/clientpositive/escape_comments.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/49882/diff/
>
>
> Testing
> -------
>
> New unit test and manually
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 49882: HIVE-14146: Column comments with "\n"
character "corrupts" table metadata
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49882/#review144109
-----------------------------------------------------------
Ship it!
Ship It!
- Sergio Pena
On July 11, 2016, 1:43 p.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49882/
> -----------------------------------------------------------
>
> (Updated July 11, 2016, 1:43 p.m.)
>
>
> Review request for hive, Aihua Xu, Sergio Pena, Szehon Ho, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-14146
> https://issues.apache.org/jira/browse/HIVE-14146
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> The patch contains:
> - MetaDataFormatUtils changes - to escape the \n in index, and column comments (table comments are already handled)
> - TextMetaDataFormatter changes - to escape the \n in database comments
> - DDLTask chages - to escape \n in show create table result
> - New query test, to test the escaping
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hive/common/util/HiveStringUtils.java 72c3fa9
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java bb43950
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java 03803bb
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java 47d67b1
> ql/src/test/queries/clientpositive/escape_comments.q PRE-CREATION
> ql/src/test/results/clientpositive/escape_comments.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/49882/diff/
>
>
> Testing
> -------
>
> New unit test and manually
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 49882: HIVE-14146: Column comments with "\n"
character "corrupts" table metadata
Posted by Peter Vary <pv...@cloudera.com>.
> On July 29, 2016, 5:16 p.m., Sahil Takiar wrote:
> > Hey Peter,
> >
> > Changes look good. One thing to note is that you may hit some conflicts when rebasing this change. As part of HIVE-12646 I modified the `DDLTask.escapeHiveCommand()` method, which is removed in this RB. The fix would just be to remove the semicolon entry from the `ESCAPE_HIVE_COMMAND` `CharSequenceTranslator`
I have checked the changes, but the HIVE-12646 patch does not affect the old hive cli (CliDriver).
This means that the escaping is still needed, or the result of the show create table command would not run in the old cli.
But if I escape it like it is needed by the CliDriver, and sent the escaped version through BeeLine, then the comment will be unescaped by the following code in SemanticAnalyzer:
case HiveParser.TOK_TABLECOMMENT:
comment = unescapeSQLString(child.getChild(0).getText());
break;
The unescapeSQLString changes the comments, so the following 2 commands create the same tables:
1., create table escape_comments_tbl9 (col1 string comment 'a\nb\'\;') comment 'a\nb\;' partitioned by (p1 string comment 'a\nb\;');
2., create table escape_comments_tbl9 (col1 string comment 'a\nb\';') comment 'a\nb;' partitioned by (p1 string comment 'a\nb;');
That is why it is ok to leave the semicolon in the ESCAPE_HIVE_COMMAND CharSequenceTranslator, and the show create table will generate the 1st version of the create table command, which is ok both for BeeLine, and for the CliDriver.
Thanks for taking the look anyway, it is good to have an extra pair of eyes for catching anything I missed.
- Peter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49882/#review144128
-----------------------------------------------------------
On July 11, 2016, 1:43 p.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49882/
> -----------------------------------------------------------
>
> (Updated July 11, 2016, 1:43 p.m.)
>
>
> Review request for hive, Aihua Xu, Sergio Pena, Szehon Ho, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-14146
> https://issues.apache.org/jira/browse/HIVE-14146
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> The patch contains:
> - MetaDataFormatUtils changes - to escape the \n in index, and column comments (table comments are already handled)
> - TextMetaDataFormatter changes - to escape the \n in database comments
> - DDLTask chages - to escape \n in show create table result
> - New query test, to test the escaping
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hive/common/util/HiveStringUtils.java 72c3fa9
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java bb43950
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java 03803bb
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java 47d67b1
> ql/src/test/queries/clientpositive/escape_comments.q PRE-CREATION
> ql/src/test/results/clientpositive/escape_comments.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/49882/diff/
>
>
> Testing
> -------
>
> New unit test and manually
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 49882: HIVE-14146: Column comments with "\n"
character "corrupts" table metadata
Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49882/#review144128
-----------------------------------------------------------
Hey Peter,
Changes look good. One thing to note is that you may hit some conflicts when rebasing this change. As part of HIVE-12646 I modified the `DDLTask.escapeHiveCommand()` method, which is removed in this RB. The fix would just be to remove the semicolon entry from the `ESCAPE_HIVE_COMMAND` `CharSequenceTranslator`
- Sahil Takiar
On July 11, 2016, 1:43 p.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49882/
> -----------------------------------------------------------
>
> (Updated July 11, 2016, 1:43 p.m.)
>
>
> Review request for hive, Aihua Xu, Sergio Pena, Szehon Ho, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-14146
> https://issues.apache.org/jira/browse/HIVE-14146
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> The patch contains:
> - MetaDataFormatUtils changes - to escape the \n in index, and column comments (table comments are already handled)
> - TextMetaDataFormatter changes - to escape the \n in database comments
> - DDLTask chages - to escape \n in show create table result
> - New query test, to test the escaping
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hive/common/util/HiveStringUtils.java 72c3fa9
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java bb43950
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java 03803bb
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java 47d67b1
> ql/src/test/queries/clientpositive/escape_comments.q PRE-CREATION
> ql/src/test/results/clientpositive/escape_comments.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/49882/diff/
>
>
> Testing
> -------
>
> New unit test and manually
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 49882: HIVE-14146: Column comments with "\n"
character "corrupts" table metadata
Posted by Peter Vary <pv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49882/
-----------------------------------------------------------
(Updated Oct. 10, 2016, 2:07 p.m.)
Review request for hive, Aihua Xu, Sergio Pena, Szehon Ho, and Vihang Karajgaonkar.
Changes
-------
Published the wrong diff before
Bugs: HIVE-14146
https://issues.apache.org/jira/browse/HIVE-14146
Repository: hive-git
Description
-------
The patch contains:
- MetaDataFormatUtils changes - to escape the \n in index, and column comments (table comments are already handled)
- TextMetaDataFormatter changes - to escape the \n in database comments
- DDLTask chages - to escape \n in show create table result
- New query test, to test the escaping
Diffs (updated)
-----
common/src/java/org/apache/hive/common/util/HiveStringUtils.java 72c3fa9
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java a59b781
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java 03803bb
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java 47d67b1
ql/src/test/queries/clientpositive/escape_comments.q PRE-CREATION
ql/src/test/results/clientnegative/alter_view_as_select_with_partition.q.out 9b84227
ql/src/test/results/clientpositive/alter_table_invalidate_column_stats.q.out f3c10ee
ql/src/test/results/clientpositive/alter_view_as_select.q.out 2d82395
ql/src/test/results/clientpositive/columnstats_part_coltype.q.out 84232b6
ql/src/test/results/clientpositive/create_like.q.out 0111c94
ql/src/test/results/clientpositive/create_view.q.out d9c1e11
ql/src/test/results/clientpositive/create_view_partitioned.q.out 15d777a
ql/src/test/results/clientpositive/create_view_translate.q.out 2789f8f
ql/src/test/results/clientpositive/create_with_constraints.q.out b040e43
ql/src/test/results/clientpositive/describe_comment_indent.q.out 5a01de1
ql/src/test/results/clientpositive/escape_comments.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/49882/diff/
Testing
-------
New unit test and manually
Thanks,
Peter Vary
Re: Review Request 49882: HIVE-14146: Column comments with "\n"
character "corrupts" table metadata
Posted by Peter Vary <pv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49882/
-----------------------------------------------------------
(Updated Oct. 10, 2016, 1:53 p.m.)
Review request for hive, Aihua Xu, Sergio Pena, Szehon Ho, and Vihang Karajgaonkar.
Bugs: HIVE-14146
https://issues.apache.org/jira/browse/HIVE-14146
Repository: hive-git
Description
-------
The patch contains:
- MetaDataFormatUtils changes - to escape the \n in index, and column comments (table comments are already handled)
- TextMetaDataFormatter changes - to escape the \n in database comments
- DDLTask chages - to escape \n in show create table result
- New query test, to test the escaping
Diffs (updated)
-----
common/src/java/org/apache/hive/common/util/HiveStringUtils.java 72c3fa9
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java ec31cf4
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java ba4f6a7
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java 47d67b1
ql/src/test/queries/clientpositive/escape_comments.q PRE-CREATION
ql/src/test/results/clientnegative/alter_view_as_select_with_partition.q.out 9b84227
ql/src/test/results/clientpositive/alter_table_invalidate_column_stats.q.out f3c10ee
ql/src/test/results/clientpositive/alter_view_as_select.q.out 2d82395
ql/src/test/results/clientpositive/columnstats_part_coltype.q.out 84232b6
ql/src/test/results/clientpositive/create_like.q.out 0111c94
ql/src/test/results/clientpositive/create_view.q.out 7281185
ql/src/test/results/clientpositive/create_view_partitioned.q.out 15d777a
ql/src/test/results/clientpositive/create_view_translate.q.out 2789f8f
ql/src/test/results/clientpositive/create_with_constraints.q.out b040e43
ql/src/test/results/clientpositive/describe_comment_indent.q.out 5a01de1
ql/src/test/results/clientpositive/escape_comments.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/49882/diff/
Testing
-------
New unit test and manually
Thanks,
Peter Vary
Re: Review Request 49882: HIVE-14146: Column comments with "\n"
character "corrupts" table metadata
Posted by Peter Vary <pv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49882/
-----------------------------------------------------------
(Updated July 11, 2016, 1:43 p.m.)
Review request for hive, Aihua Xu, Sergio Pena, Szehon Ho, and Vihang Karajgaonkar.
Changes
-------
Incorporated Aihua's recommendations
Bugs: HIVE-14146
https://issues.apache.org/jira/browse/HIVE-14146
Repository: hive-git
Description
-------
The patch contains:
- MetaDataFormatUtils changes - to escape the \n in index, and column comments (table comments are already handled)
- TextMetaDataFormatter changes - to escape the \n in database comments
- DDLTask chages - to escape \n in show create table result
- New query test, to test the escaping
Diffs (updated)
-----
common/src/java/org/apache/hive/common/util/HiveStringUtils.java 72c3fa9
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java bb43950
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java 03803bb
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java 47d67b1
ql/src/test/queries/clientpositive/escape_comments.q PRE-CREATION
ql/src/test/results/clientpositive/escape_comments.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/49882/diff/
Testing
-------
New unit test and manually
Thanks,
Peter Vary