Re: Review Request 60753: Add HLL as an alternative to FM sketch to compute stats

2017-07-13 Thread Andrew Sherman 

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



Comment "Copyright 2017 Prasanth Jayachandran" in HyperLogLogUtils.java should 
probably be removed

- Andrew Sherman


On July 13, 2017, 12:12 a.m., pengcheng xiong wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60753/
> ---
> 
> (Updated July 13, 2017, 12:12 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16966
> 
> 
> Diffs
> -
> 
>   common/pom.xml 023f084511 
>   
> common/src/java/org/apache/hadoop/hive/common/ndv/NumDistinctValueEstimatorFactory.java
>  PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/ndv/hll/HLLConstants.java 
> PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/ndv/hll/HLLDenseRegister.java 
> PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/ndv/hll/HLLRegister.java 
> PRE-CREATION 
>   
> common/src/java/org/apache/hadoop/hive/common/ndv/hll/HLLSparseRegister.java 
> PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/ndv/hll/HyperLogLog.java 
> PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/ndv/hll/HyperLogLogUtils.java 
> PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5700fb9325 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/NumDistinctValueEstimator.java
>  92f9a845e3 
>   metastore/src/java/org/apache/hadoop/hive/metastore/hbase/StatsCache.java 
> 18f8afc9ad 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/ColumnStatsAggregator.java
>  31955b4363 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/ColumnStatsAggregatorFactory.java
>  daf85692eb 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/DecimalColumnStatsAggregator.java
>  36b2c9c56b 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/DoubleColumnStatsAggregator.java
>  a88ef84e5c 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/LongColumnStatsAggregator.java
>  8ac6561aec 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/StringColumnStatsAggregator.java
>  2aa4046a46 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/merge/ColumnStatsMerger.java
>  33c7e3e52c 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/merge/ColumnStatsMergerFactory.java
>  fe890e4e27 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/merge/DateColumnStatsMerger.java
>  3179b23438 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/merge/DecimalColumnStatsMerger.java
>  c13add9d9c 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/merge/DoubleColumnStatsMerger.java
>  fbdba24b0a 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/merge/LongColumnStatsMerger.java
>  ac65590505 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/merge/StringColumnStatsMerger.java
>  41587477d3 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseAggregateStatsExtrapolation.java
>  99ce96ca0d 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseAggregateStatsNDVUniformDist.java
>  87b1ac870d 
>   pom.xml 1b8963274e 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java 
> 0a5cf00c44 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java 76f7daeb1b 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/DecimalNumDistinctValueEstimator.java
>  a05906edfa 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/DoubleNumDistinctValueEstimator.java
>  e76fc74dbc 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFComputeStats.java
>  2ebfcb2360 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/LongNumDistinctValueEstimator.java
>  1c197a028a 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/NumDistinctValueEstimator.java
>  fa70f49857 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/StringNumDistinctValueEstimator.java
>  601901c163 
>   ql/src/test/queries/clientpositive/compute_stats_date.q 09128f6fb9 
>   ql/src/test/queries/clientpositive/hll.q PRE-CREATION 
>   ql/src/test/results/clientpositive/alte

Re: [DISCUSS] Separating out the metastore as its own TLP

2017-07-11 Thread Andrew Sherman 
On Fri, Jun 30, 2017 at 5:05 PM, Owen O'Malley 
wrote:

> On Fri, Jun 30, 2017 at 3:26 PM, Chao Sun  wrote:
>
> > and maybe a different project name?
> >
>
> Yes, it certainly needs a new name. I'd like to suggest Riven.
>
> .. Owen
>

How about "Flora"?

(Flora is the protagonist of The Bees by Laline Paull)

-Andrew

Re: Review Request 60445: HIVE-16935: Hive should strip comments from input before choosing which CommandProcessor to run.

2017-07-05 Thread Andrew Sherman 

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


Changes
---

Use Guava's Splitter


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 (updated)
-

  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/

Changes: https://reviews.apache.org/r/60445/diff/3-4/


Testing
---

Added new test case.
Hand tested with Hue and Jdbc.


Thanks,

Andrew Sherman

Re: Review Request 60445: HIVE-16935: Hive should strip comments from input before choosing which CommandProcessor to run.

2017-06-30 Thread Andrew Sherman 

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

(Updated June 30, 2017, 4:18 p.m.)


Review request for hive and Sahil Takiar.


Bugs: HIVE-16935
https://issues.apache.org/jira/browse/HIVE-16935


Repository: hive-git


Description (updated)
---

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 (updated)
-

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

Changes: https://reviews.apache.org/r/60445/diff/2-3/


Testing
---

Added new test case.
Hand tested with Hue and Jdbc.


Thanks,

Andrew Sherman

Re: Review Request 60445: HIVE-16935: Hive should strip comments from input before choosing which CommandProcessor to run.

2017-06-29 Thread Andrew Sherman 

---
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 (updated)
---

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 (updated)
-

  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/

Changes: https://reviews.apache.org/r/60445/diff/1-2/


Testing
---

Added new test case.
Hand tested with Hue and Jdbc.


Thanks,

Andrew Sherman

Re: Review Request 60445: HIVE-16935: Hive should strip comments from input before choosing which CommandProcessor to run.

2017-06-29 Thread Andrew Sherman 


> On June 28, 2017, 10:39 a.m., Peter Vary wrote:
> > Hi,
> > 
> > Thanks for the change.
> > Overall looks good.
> > After running Yetus, it come up with the following warnings:
> > 
> > ./common/src/java/org/apache/hive/common/util/HiveStringUtils.java:1077:  
> > /**: warning: First sentence should end with a period.
> > ./common/src/java/org/apache/hive/common/util/HiveStringUtils.java:1088:
> > int[] startQuote = { -1 };:25: warning: '{' is followed by whitespace.
> > ./common/src/java/org/apache/hive/common/util/HiveStringUtils.java:1105:
> > if (line == null || line.isEmpty()): warning: 'if' construct must use '{}'s.
> > ./common/src/java/org/apache/hive/common/util/HiveStringUtils.java:1107:
> > if (startQuote[0] == -1 && isComment(line)): warning: 'if' construct must 
> > use '{}'s.
> > ./cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java:126:String 
> > cmd_trimmed = removeComments(cmd).trim();:12: warning: Name 'cmd_trimmed' 
> > must match pattern '^[a-z][a-zA-Z0-9]*$'.
> > 
> > I personally do not like to import methods:
> > https://stackoverflow.com/questions/420791/what-is-a-good-use-case-for-static-import-of-methods
> > 
> > Thanks for the patch,
> > Peter

Thanks for the review.
Thanks for letting me know about Yetus. I was able to follow the instructiona 
at https://cwiki.apache.org/confluence/display/Hive/Running+Yetus
I have fixed the checkstyle problems except for the name 'cmd_trimmed'. 
Changing this causes cascading checkstyle changes which expand the scope of the 
fix too far.
I reduced the usage of static imports but kept it in the test where it makes 
the test more readable.


> On June 28, 2017, 10:39 a.m., Peter Vary wrote:
> > common/src/java/org/apache/hive/common/util/HiveStringUtils.java
> > Lines 1104 (patched)
> > <https://reviews.apache.org/r/60445/diff/1/?file=1763215#file1763215line1104>
> >
> > nit: do we need this as public?

Yes, it is called from org.apache.hive.beeline.Commands


- Andrew


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


On June 26, 2017, 8:35 p.m., Andrew Sherman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60445/
> ---
> 
> (Updated June 26, 2017, 8:35 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.
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/Commands.java 3b2d72ed79 
>   beeline/src/test/org/apache/hive/beeline/TestCommands.java 04c939a04c 
>   cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 27fd66d35e 
>   common/src/java/org/apache/hive/common/util/HiveStringUtils.java 4a6413a7c3 
>   common/src/test/org/apache/hive/common/util/TestHiveStringUtils.java 
> 6bd7037152 
>   
> ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java
>  21bdcf4443 
>   ql/src/test/queries/clientpositive/set_processor_namespaces.q 612807f0c8 
>   ql/src/test/results/clientpositive/set_processor_namespaces.q.out 
> c05ce4d61d 
>   
> service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java
>  2dd90b69b3 
> 
> 
> Diff: https://reviews.apache.org/r/60445/diff/1/
> 
> 
> Testing
> ---
> 
> Added new test case.
> Hand tested with Hue and Jdbc.
> 
> 
> Thanks,
> 
> Andrew Sherman
> 
>

Re: Review Request 60445: HIVE-16935: Hive should strip comments from input before choosing which CommandProcessor to run.

2017-06-29 Thread Andrew Sherman 


> On June 28, 2017, 5:18 a.m., Sahil Takiar wrote:
> > common/src/java/org/apache/hive/common/util/HiveStringUtils.java
> > Lines 1083 (patched)
> > <https://reviews.apache.org/r/60445/diff/1/?file=1763215#file1763215line1083>
> >
> > is this copied from somewhere, or is this logic new?

This code is new.


> On June 28, 2017, 5:18 a.m., Sahil Takiar wrote:
> > common/src/java/org/apache/hive/common/util/HiveStringUtils.java
> > Lines 1104 (patched)
> > <https://reviews.apache.org/r/60445/diff/1/?file=1763215#file1763215line1104>
> >
> > are there any changes to this method, or was it just moved from 
> > `Commands.java`

This code was moved. Peter requests updated javadoc so I will change that in  
the next revision.


> On June 28, 2017, 5:18 a.m., Sahil Takiar wrote:
> > common/src/java/org/apache/hive/common/util/HiveStringUtils.java
> > Lines 1136 (patched)
> > <https://reviews.apache.org/r/60445/diff/1/?file=1763215#file1763215line1136>
> >
> > new logic, or copied from beeline?

Moved from beeline


> On June 28, 2017, 5:18 a.m., Sahil Takiar wrote:
> > common/src/test/org/apache/hive/common/util/TestHiveStringUtils.java
> > Lines 113-115 (patched)
> > <https://reviews.apache.org/r/60445/diff/1/?file=1763216#file1763216line113>
> >
> > nit: remove extra lines

Fixed in next patch


- Andrew


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


On June 26, 2017, 8:35 p.m., Andrew Sherman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60445/
> ---
> 
> (Updated June 26, 2017, 8:35 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.
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/Commands.java 3b2d72ed79 
>   beeline/src/test/org/apache/hive/beeline/TestCommands.java 04c939a04c 
>   cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 27fd66d35e 
>   common/src/java/org/apache/hive/common/util/HiveStringUtils.java 4a6413a7c3 
>   common/src/test/org/apache/hive/common/util/TestHiveStringUtils.java 
> 6bd7037152 
>   
> ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java
>  21bdcf4443 
>   ql/src/test/queries/clientpositive/set_processor_namespaces.q 612807f0c8 
>   ql/src/test/results/clientpositive/set_processor_namespaces.q.out 
> c05ce4d61d 
>   
> service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java
>  2dd90b69b3 
> 
> 
> Diff: https://reviews.apache.org/r/60445/diff/1/
> 
> 
> Testing
> ---
> 
> Added new test case.
> Hand tested with Hue and Jdbc.
> 
> 
> Thanks,
> 
> Andrew Sherman
> 
>

[jira] [Created] (HIVE-16991) HiveMetaStoreClient needs a 2-arg constructor for backwards compatibility

2017-06-29 Thread Andrew Sherman (JIRA) 
Andrew Sherman created HIVE-16991:
-

 Summary: HiveMetaStoreClient needs a 2-arg constructor for 
backwards compatibility
 Key: HIVE-16991
 URL: https://issues.apache.org/jira/browse/HIVE-16991
 Project: Hive
  Issue Type: Bug
Reporter: Andrew Sherman
Assignee: Andrew Sherman


Some client code that is not easy to change uses a 2-arg constructor on 
HiveMetaStoreClient.
It is trivial and safe to add this constructor:

{{public HiveMetaStoreClient(HiveConf conf, HiveMetaHookLoader hookLoader) 
throws MetaException {
this(conf, hookLoader, true);
}}}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Review Request 60445: HIVE-16935: Hive should strip comments from input before choosing which CommandProcessor to run.

2017-06-26 Thread Andrew Sherman 

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

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.


Diffs
-

  beeline/src/java/org/apache/hive/beeline/Commands.java 3b2d72ed79 
  beeline/src/test/org/apache/hive/beeline/TestCommands.java 04c939a04c 
  cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 27fd66d35e 
  common/src/java/org/apache/hive/common/util/HiveStringUtils.java 4a6413a7c3 
  common/src/test/org/apache/hive/common/util/TestHiveStringUtils.java 
6bd7037152 
  
ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java
 21bdcf4443 
  ql/src/test/queries/clientpositive/set_processor_namespaces.q 612807f0c8 
  ql/src/test/results/clientpositive/set_processor_namespaces.q.out c05ce4d61d 
  
service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java
 2dd90b69b3 


Diff: https://reviews.apache.org/r/60445/diff/1/


Testing
---

Added new test case.
Hand tested with Hue and Jdbc.


Thanks,

Andrew Sherman

[jira] [Created] (HIVE-16935) Hive should strip comments from input before choosing which CommandProcessor to run.

2017-06-21 Thread Andrew Sherman (JIRA) 
Andrew Sherman created HIVE-16935:
-

 Summary: Hive should strip comments from input before choosing 
which CommandProcessor to run.
 Key: HIVE-16935
 URL: https://issues.apache.org/jira/browse/HIVE-16935
 Project: Hive
  Issue Type: Bug
Reporter: Andrew Sherman
Assignee: Andrew Sherman


While using Beeswax, Hue fails to execute statement with following error:

Error while compiling statement: FAILED: ParseException line 3:4 missing 
KW_ROLE at 'a' near 'a' line 3:5 missing EOF at '=' near 'a'

{quote}
-- comment
SET a=1;
SELECT 1;
{quote}

The same code works in Beeline and in Impala.
The same code fails in CliDriver 
 
h2. Background

Hive deals with sql comments (“-- to end of line”) in different places.
Some clients attempt to strip comments. For example BeeLine was recently 
enhanced in https://issues.apache.org/jira/browse/HIVE-13864 to strip comments 
from multi-line commands before they are executed.
Other clients such as Hue or Jdbc do not strip comments before sending text.
Some tests such as TestCliDriver strip comments before running tests.
When Hive gets a command the CommandProcessorFactory looks at the text to 
determine which CommandProcessor should handle the command. In the bug case the 
correct CommandProcessor is SetProcessor, but the comments confuse the 
CommandProcessorFactory and so the command is treated as sql. Hive’s sql parser 
understands and ignores comments, but it does not understand the set commands 
usually handled by SetProcessor and so we get the ParseException shown above.
 




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

<    1   2