Mailing-List: contact dev-help@hive.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hive.apache.org
Content-Type: multipart/alternative;
 boundary="===============6847280938012666561=="
MIME-Version: 1.0
Subject: Re: Review Request 60445: HIVE-16935: Hive should strip comments from
 input before choosing which CommandProcessor to run.
From: Peter Vary <pvary@cloudera.com>
To: Sahil Takiar <takiar.sahil@gmail.com>
Cc: Peter Vary <pvary@cloudera.com>, hive <dev@hive.apache.org>,
 Andrew Sherman <asherman@cloudera.com>
Date: Fri, 30 Jun 2017 15:17:08 -0000
Message-ID: <20170630151708.65120.16073@reviews-vm2.apache.org>
Auto-Submitted: auto-generated
Sender: Peter Vary <noreply@reviews.apache.org>
References: <20170630003040.64391.50205@reviews-vm2.apache.org>
In-Reply-To: <20170630003040.64391.50205@reviews-vm2.apache.org>
Reply-To: Peter Vary <pvary@cloudera.com>
archived-at: Fri, 30 Jun 2017 15:17:21 -0000

--===============6847280938012666561==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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


Thanks for the fast update! LGTM, just one little nit :)

Peter


common/src/java/org/apache/hive/common/util/HiveStringUtils.java
Lines 1101-1103 (original), 1101-1103 (patched)
<https://reviews.apache.org/r/60445/#comment254036>

    nit: I think your new javadoc is good, so we can remove this. What do you think?


- Peter Vary


On June 30, 2017, 12:30 a.m., Andrew Sherman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60445/
> -----------------------------------------------------------
> 
> (Updated June 30, 2017, 12:30 a.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/2/
> 
> 
> Testing
> -------
> 
> Added new test case.
> Hand tested with Hue and Jdbc.
> 
> 
> Thanks,
> 
> Andrew Sherman
> 
>


--===============6847280938012666561==--