> 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
>
>
