hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cheng xu <cheng.a...@intel.com>
Subject Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script
Date Wed, 21 Sep 2016 05:45:03 GMT

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




jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 237)
<https://reviews.apache.org/r/51695/#comment217505>

    No need for "this."



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 250)
<https://reviews.apache.org/r/51695/#comment217501>

    LOG.error



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 251)
<https://reviews.apache.org/r/51695/#comment217502>

    Please throw SQLException.



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 256)
<https://reviews.apache.org/r/51695/#comment217511>

    public method?



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 289)
<https://reviews.apache.org/r/51695/#comment217510>

    private method?



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 295)
<https://reviews.apache.org/r/51695/#comment217509>

    Please use if-else since you have only switch case.



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 747)
<https://reviews.apache.org/r/51695/#comment217503>

    Do we need this method? It's private.



jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 48)
<https://reviews.apache.org/r/51695/#comment217508>

    Two space indents please.



jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 53)
<https://reviews.apache.org/r/51695/#comment217506>

    This is not a negative case.
    "#negative cases:"->"#Some comments"



jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 55)
<https://reviews.apache.org/r/51695/#comment217507>

    Please add the case "#show tables; show/n tables"



jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 75)
<https://reviews.apache.org/r/51695/#comment217504>

    Assert.fail("Test was failed due to " + e);


- cheng xu


On Sept. 21, 2016, 11:47 a.m., Jianguo Tian wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51695/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2016, 11:47 a.m.)
> 
> 
> Review request for hive and cheng xu.
> 
> 
> Bugs: HIVE-5867
>     https://issues.apache.org/jira/browse/HIVE-5867
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-5867: JDBC driver and beeline should support executing an initial SQL script
> 
> 
> Diffs
> -----
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java ad96a6466dd1aadab71fc261f55be4639dcbe2bf

>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 3161566994d6c6e01de9d88a6e87295684619ffa

>   jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51695/diff/
> 
> 
> Testing
> -------
> 
> TestInitSQL.java is JUnit test class which will test method initSql() in HiveConnection.java.
> 
> 
> Thanks,
> 
> Jianguo Tian
> 
>


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