You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Sahil Takiar <ta...@gmail.com> on 2016/07/12 04:04:48 UTC
Re: Review Request 49655: HIVE-12646: beeline and HIVE CLI do not
parse ; in quote properly
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49655/
-----------------------------------------------------------
(Updated July 12, 2016, 4:04 a.m.)
Review request for hive, Sergio Pena and Yongzhi Chen.
Bugs: HIVE-12646
https://issues.apache.org/jira/browse/HIVE-12646
Repository: hive-git
Description
-------
HIVE-12646: beeline and HIVE CLI do not parse ; in quote properly
Approach:
* Modified the `Commands.execute(...)` command to iterate throught the given input line character by character
* It looks for single and double quotes in order to track when the iterator is inside a quotation block
* If the iterator is inside a quotation block and it finds a semicolon, it ignores it, otherwise it treats it as it normally would
* Moved the logic for parsing the line into a helper method called `getCmList(...)` which is responsible for returning a `List` of commands that need to be run
Diffs
-----
beeline/src/java/org/apache/hive/beeline/Commands.java 3a204c0
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java ecfeddb
Diff: https://reviews.apache.org/r/49655/diff/
Testing
-------
Add a unit tests which checks that Beeline can successfully run queries that contain semi-colons inside quotation blocks. Confirmed existing unit tests pass.
Thanks,
Sahil Takiar
Re: Review Request 49655: HIVE-12646: beeline and HIVE CLI do not
parse ; in quote properly
Posted by Sahil Takiar <ta...@gmail.com>.
> On July 14, 2016, 4:42 p.m., Sergio Pena wrote:
> > The patch looks good. Could you add more tests to validate different cases?
> >
> > For instance:
> > select ';'
> > select '";"'
> > select "';'"
> > select "\';\'"
> > select "\";\""
> > select '\';\''
Added another test to cover these cases as well as the one that Peter mentioned.
- Sahil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49655/#review142247
-----------------------------------------------------------
On July 19, 2016, 7:45 p.m., Sahil Takiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49655/
> -----------------------------------------------------------
>
> (Updated July 19, 2016, 7:45 p.m.)
>
>
> Review request for hive, Sergio Pena and Yongzhi Chen.
>
>
> Bugs: HIVE-12646
> https://issues.apache.org/jira/browse/HIVE-12646
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-12646: beeline and HIVE CLI do not parse ; in quote properly
>
> Approach:
>
> * Modified the `Commands.execute(...)` command to iterate throught the given input line character by character
> * It looks for single and double quotes in order to track when the iterator is inside a quotation block
> * If the iterator is inside a quotation block and it finds a semicolon, it ignores it, otherwise it treats it as it normally would
> * Moved the logic for parsing the line into a helper method called `getCmList(...)` which is responsible for returning a `List` of commands that need to be run
>
>
> Diffs
> -----
>
> beeline/src/java/org/apache/hive/beeline/Commands.java 387861b
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java ecfeddb
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 2b8d6a7
>
> Diff: https://reviews.apache.org/r/49655/diff/
>
>
> Testing
> -------
>
> Add a unit tests which checks that Beeline can successfully run queries that contain semi-colons inside quotation blocks. Confirmed existing unit tests pass.
>
>
> Thanks,
>
> Sahil Takiar
>
>
Re: Review Request 49655: HIVE-12646: beeline and HIVE CLI do not
parse ; in quote properly
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49655/#review142247
-----------------------------------------------------------
The patch looks good. Could you add more tests to validate different cases?
For instance:
select ';'
select '";"'
select "';'"
select "\';\'"
select "\";\""
select '\';\''
- Sergio Pena
On July 12, 2016, 4:04 a.m., Sahil Takiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49655/
> -----------------------------------------------------------
>
> (Updated July 12, 2016, 4:04 a.m.)
>
>
> Review request for hive, Sergio Pena and Yongzhi Chen.
>
>
> Bugs: HIVE-12646
> https://issues.apache.org/jira/browse/HIVE-12646
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-12646: beeline and HIVE CLI do not parse ; in quote properly
>
> Approach:
>
> * Modified the `Commands.execute(...)` command to iterate throught the given input line character by character
> * It looks for single and double quotes in order to track when the iterator is inside a quotation block
> * If the iterator is inside a quotation block and it finds a semicolon, it ignores it, otherwise it treats it as it normally would
> * Moved the logic for parsing the line into a helper method called `getCmList(...)` which is responsible for returning a `List` of commands that need to be run
>
>
> Diffs
> -----
>
> beeline/src/java/org/apache/hive/beeline/Commands.java 3a204c0
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java ecfeddb
>
> Diff: https://reviews.apache.org/r/49655/diff/
>
>
> Testing
> -------
>
> Add a unit tests which checks that Beeline can successfully run queries that contain semi-colons inside quotation blocks. Confirmed existing unit tests pass.
>
>
> Thanks,
>
> Sahil Takiar
>
>
Re: Review Request 49655: HIVE-12646: beeline and HIVE CLI do not
parse ; in quote properly
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49655/#review143090
-----------------------------------------------------------
Ship it!
Ship It!
- Sergio Pena
On July 19, 2016, 7:45 p.m., Sahil Takiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49655/
> -----------------------------------------------------------
>
> (Updated July 19, 2016, 7:45 p.m.)
>
>
> Review request for hive, Sergio Pena and Yongzhi Chen.
>
>
> Bugs: HIVE-12646
> https://issues.apache.org/jira/browse/HIVE-12646
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-12646: beeline and HIVE CLI do not parse ; in quote properly
>
> Approach:
>
> * Modified the `Commands.execute(...)` command to iterate throught the given input line character by character
> * It looks for single and double quotes in order to track when the iterator is inside a quotation block
> * If the iterator is inside a quotation block and it finds a semicolon, it ignores it, otherwise it treats it as it normally would
> * Moved the logic for parsing the line into a helper method called `getCmList(...)` which is responsible for returning a `List` of commands that need to be run
>
>
> Diffs
> -----
>
> beeline/src/java/org/apache/hive/beeline/Commands.java 387861b
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java ecfeddb
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 2b8d6a7
>
> Diff: https://reviews.apache.org/r/49655/diff/
>
>
> Testing
> -------
>
> Add a unit tests which checks that Beeline can successfully run queries that contain semi-colons inside quotation blocks. Confirmed existing unit tests pass.
>
>
> Thanks,
>
> Sahil Takiar
>
>
Re: Review Request 49655: HIVE-12646: beeline and HIVE CLI do not
parse ; in quote properly
Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49655/
-----------------------------------------------------------
(Updated July 19, 2016, 7:45 p.m.)
Review request for hive, Sergio Pena and Yongzhi Chen.
Changes
-------
Addressing comments. Added another test that covers 7 more cases. Updated logic to handle escape quotation marks. Removed escaping of ; in DDLTask done for `SHOW CREATE` queries, as it is no longer necessary.
Bugs: HIVE-12646
https://issues.apache.org/jira/browse/HIVE-12646
Repository: hive-git
Description
-------
HIVE-12646: beeline and HIVE CLI do not parse ; in quote properly
Approach:
* Modified the `Commands.execute(...)` command to iterate throught the given input line character by character
* It looks for single and double quotes in order to track when the iterator is inside a quotation block
* If the iterator is inside a quotation block and it finds a semicolon, it ignores it, otherwise it treats it as it normally would
* Moved the logic for parsing the line into a helper method called `getCmList(...)` which is responsible for returning a `List` of commands that need to be run
Diffs (updated)
-----
beeline/src/java/org/apache/hive/beeline/Commands.java 387861b
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java ecfeddb
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 2b8d6a7
Diff: https://reviews.apache.org/r/49655/diff/
Testing
-------
Add a unit tests which checks that Beeline can successfully run queries that contain semi-colons inside quotation blocks. Confirmed existing unit tests pass.
Thanks,
Sahil Takiar
Re: Review Request 49655: HIVE-12646: beeline and HIVE CLI do not
parse ; in quote properly
Posted by Sahil Takiar <ta...@gmail.com>.
> On July 13, 2016, 8:37 a.m., Peter Vary wrote:
> > Thanks for your patch!
> > This was a very annoying "feature" :)
> >
> > As discussed offline, please look at DDLTask.java where the SHOW CREATE TABLE command is implemented. There is an escapeHiveCommand method, which escapes the result which is expected to be copy/pasted back to command line and executed without change.
> >
> > Another thing your patch should consider, that the comment fields could contain escaped ' characters, like the one below:
> >
> > ```
> > create table escape_comments_tbl1 (col1 string comment 'ab\';\');
> > ```
> >
> > Thanks,
> > Peter
Thanks for pointing out the logic in DDLTask Peter. I hadn't thought about that. My solution was to basically remove the escaping of the `;` done in DDLTask. It no longer necessary since the create table queries can now handle `;` as long as they are inside quotation marks (I checked and all uses of the escapeHiveCommand return output that is enclosed in quotation marks). So the create table queries should work as expected. The old behavior is still preserved e.g. `'ab\;cd'` still works (there were existing test for this). It seems Hive internally strips the escape characters for characters that don't need to be escaped (ref: `BaseSemanticAnalyzer.unescapeSQLString(String)`.
- Sahil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49655/#review142036
-----------------------------------------------------------
On July 19, 2016, 7:45 p.m., Sahil Takiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49655/
> -----------------------------------------------------------
>
> (Updated July 19, 2016, 7:45 p.m.)
>
>
> Review request for hive, Sergio Pena and Yongzhi Chen.
>
>
> Bugs: HIVE-12646
> https://issues.apache.org/jira/browse/HIVE-12646
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-12646: beeline and HIVE CLI do not parse ; in quote properly
>
> Approach:
>
> * Modified the `Commands.execute(...)` command to iterate throught the given input line character by character
> * It looks for single and double quotes in order to track when the iterator is inside a quotation block
> * If the iterator is inside a quotation block and it finds a semicolon, it ignores it, otherwise it treats it as it normally would
> * Moved the logic for parsing the line into a helper method called `getCmList(...)` which is responsible for returning a `List` of commands that need to be run
>
>
> Diffs
> -----
>
> beeline/src/java/org/apache/hive/beeline/Commands.java 387861b
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java ecfeddb
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 2b8d6a7
>
> Diff: https://reviews.apache.org/r/49655/diff/
>
>
> Testing
> -------
>
> Add a unit tests which checks that Beeline can successfully run queries that contain semi-colons inside quotation blocks. Confirmed existing unit tests pass.
>
>
> Thanks,
>
> Sahil Takiar
>
>
Re: Review Request 49655: HIVE-12646: beeline and HIVE CLI do not
parse ; in quote properly
Posted by Peter Vary <pv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49655/#review142036
-----------------------------------------------------------
Thanks for your patch!
This was a very annoying "feature" :)
As discussed offline, please look at DDLTask.java where the SHOW CREATE TABLE command is implemented. There is an escapeHiveCommand method, which escapes the result which is expected to be copy/pasted back to command line and executed without change.
Another thing your patch should consider, that the comment fields could contain escaped ' characters, like the one below:
```
create table escape_comments_tbl1 (col1 string comment 'ab\';\');
```
Thanks,
Peter
- Peter Vary
On July 12, 2016, 4:04 a.m., Sahil Takiar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49655/
> -----------------------------------------------------------
>
> (Updated July 12, 2016, 4:04 a.m.)
>
>
> Review request for hive, Sergio Pena and Yongzhi Chen.
>
>
> Bugs: HIVE-12646
> https://issues.apache.org/jira/browse/HIVE-12646
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-12646: beeline and HIVE CLI do not parse ; in quote properly
>
> Approach:
>
> * Modified the `Commands.execute(...)` command to iterate throught the given input line character by character
> * It looks for single and double quotes in order to track when the iterator is inside a quotation block
> * If the iterator is inside a quotation block and it finds a semicolon, it ignores it, otherwise it treats it as it normally would
> * Moved the logic for parsing the line into a helper method called `getCmList(...)` which is responsible for returning a `List` of commands that need to be run
>
>
> Diffs
> -----
>
> beeline/src/java/org/apache/hive/beeline/Commands.java 3a204c0
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java ecfeddb
>
> Diff: https://reviews.apache.org/r/49655/diff/
>
>
> Testing
> -------
>
> Add a unit tests which checks that Beeline can successfully run queries that contain semi-colons inside quotation blocks. Confirmed existing unit tests pass.
>
>
> Thanks,
>
> Sahil Takiar
>
>