hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vihang Karajgaonkar <vih...@cloudera.com>
Subject Re: Review Request 60445: HIVE-16935: Hive should strip comments from input before choosing which CommandProcessor to run.
Date Thu, 06 Jul 2017 20:26:53 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60445/#review179819
-----------------------------------------------------------


Fix it, then Ship it!





common/src/java/org/apache/hive/common/util/HiveStringUtils.java
Lines 1149-1150 (patched)
<https://reviews.apache.org/r/60445/#comment254682>

    I didn't know that BeeLine supported "#" as a comment prefix (and I don't see any documentation
for that on wiki as well). While BeeLine may be supporting it and we should probably not change
that now, I am not sure if we should keep it as well here since now it will apply to all the
other JDBC clients. Should we rather stick to the SQL92 comment prefix only here and let BeeLine
keep using "#" in addition to "--"? What do you think?



common/src/test/org/apache/hive/common/util/TestHiveStringUtils.java
Lines 112 (patched)
<https://reviews.apache.org/r/60445/#comment254672>

    nit, spellcheck comments


- Vihang Karajgaonkar


On July 5, 2017, 6:09 p.m., Andrew Sherman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60445/
> -----------------------------------------------------------
> 
> (Updated July 5, 2017, 6:09 p.m.)
> 
> 
> Review request for hive and Sahil Takiar.
> 
> 
> Bugs: HIVE-16935
>     https://issues.apache.org/jira/browse/HIVE-16935
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> We strip sql comments from a command string. The stripped command is use to determine
which
> CommandProcessor will execute the command. If the CommandProcessorFactory does not select
a special
> CommandProcessor then we execute the original unstripped command so that the sql parser
can remove comments.
> Move BeeLine's comment stripping code to HiveStringUtils and change BeeLine to call it
from there
> Add a better test with separate tokens for "set role" in TestCommandProcessorFactory.
> Add a test case for comment removal in set_processor_namespaces.q using an indented comment
as
> unindented comments are removed by the test driver.
> 
> Change-Id: I166dc1e7588ec9802ba373d88e69e716aecd33c2
> 
> 
> Diffs
> -----
> 
>   beeline/src/java/org/apache/hive/beeline/Commands.java 3b2d72ed79771e6198e62c47060a7f80665dbcb2

>   beeline/src/test/org/apache/hive/beeline/TestCommands.java 04c939a04c7a56768286743c2bb9c9797507e3aa

>   cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 27fd66d35ea89b0de0d17763625fbf564584fcca

>   common/src/java/org/apache/hive/common/util/HiveStringUtils.java 4a6413a7c376ffb4de6d20d24707ac5bf89ebc0c

>   common/src/test/org/apache/hive/common/util/TestHiveStringUtils.java 6bd7037152c6f809daec8af42708693c05fe00cf

>   ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java 21bdcf44436a02b11f878fa439e916d4b55ac63d

>   ql/src/test/queries/clientpositive/set_processor_namespaces.q 612807f0c871b1881446d088e1c2c399d1afe970

>   ql/src/test/results/clientpositive/set_processor_namespaces.q.out c05ce4d61d00a9ee6671d97f2fd178f18d44cc8c

>   service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java
2dd90b69b3bf789b1a3928129cf801b17884033f 
> 
> 
> Diff: https://reviews.apache.org/r/60445/diff/4/
> 
> 
> Testing
> -------
> 
> Added new test case.
> Hand tested with Hue and Jdbc.
> 
> 
> Thanks,
> 
> Andrew Sherman
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message