hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Xuefu Zhang" <xzh...@cloudera.com>
Subject Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
Date Thu, 25 Jun 2015 21:54:36 GMT

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



beeline/src/java/org/apache/hive/beeline/Commands.java (line 721)
<https://reviews.apache.org/r/35107/#comment142021>

    Nit: Keep @Override in a separate line.



beeline/src/java/org/apache/hive/beeline/Commands.java (line 722)
<https://reviews.apache.org/r/35107/#comment142022>

    I think getHiveVariables() is in the same class, which can be directly accessed.



beeline/src/java/org/apache/hive/beeline/Commands.java (line 760)
<https://reviews.apache.org/r/35107/#comment142023>

    I think getHiveVariables() can be private.



beeline/src/java/org/apache/hive/beeline/Commands.java (line 772)
<https://reviews.apache.org/r/35107/#comment142024>

    getHiveConf() can be proviate. Remove the caller from BeeLine.java.



beeline/src/java/org/apache/hive/beeline/Commands.java (line 850)
<https://reviews.apache.org/r/35107/#comment142030>

    This used to call BeeLine.executeFile() now you have a new implementation. I don't quite
follow why. Regardless, I see two problems.
    1. we should remove the 2nd argument as all callers provide "false".
    2. Some part of the code in executeFile() is to fix some problem with the last line in
the script file. Will your new implementation catches that?
    3. It's my understanding that in Hive CLI compatible mode, commands in source file should
follow Hive CLI syntax, while in normal mode, it should follow Beeline syntax. Is this what's
done here?


- Xuefu Zhang


On June 25, 2015, 5:54 a.m., cheng xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35107/
> -----------------------------------------------------------
> 
> (Updated June 25, 2015, 5:54 a.m.)
> 
> 
> Review request for hive, chinna and Xuefu Zhang.
> 
> 
> Bugs: HIVE-6791
>     https://issues.apache.org/jira/browse/HIVE-6791
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Summary:
> 1) move the beeline-cli convertor to the place where cli is executed(class **Commands**)
> 2) support substitution for source command
> 3) add some unit test for substitution
> 4) add one way to get the configuration from HS2
> 
> 
> Diffs
> -----
> 
>   beeline/src/java/org/apache/hive/beeline/BeeLine.java b7d2f2e 
>   beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3 
>   beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030 
>   cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c 
>   common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION

>   common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION

>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f

>   ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96 
>   ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53

>   ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168 
>   ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82

>   ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d 
>   ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c 
>   service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b 
> 
> Diff: https://reviews.apache.org/r/35107/diff/
> 
> 
> Testing
> -------
> 
> Unit test passed
> 
> 
> Thanks,
> 
> cheng xu
> 
>


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