Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[Jianguo Tian]

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

(Updated Sept. 26, 2016, 6:21 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 (updated)
-

  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/TestJdbcDriver.java PRE-CREATION 

Diff: https://reviews.apache.org/r/51695/diff/


Testing
---

TestRunInitialSQL.java is a JUnit test class which can test parseInitFile() 
method of HiveConnection.java. I test some positive cases and negative cases to 
look that if these cases could be parse into SQL statement which could be 
executed successfully.


Thanks,

Jianguo Tian

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[cheng xu]

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




jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 100)


Since this import is not needed.



jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java (line 39)


Can we rename this class to TestJdbcDriver?


- cheng xu


On Sept. 23, 2016, 3:31 p.m., Jianguo Tian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51695/
> ---
> 
> (Updated Sept. 23, 2016, 3:31 p.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/TestRunInitialSQL.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51695/diff/
> 
> 
> Testing
> ---
> 
> TestRunInitialSQL.java is a JUnit test class which can test parseInitFile() 
> method of HiveConnection.java. I test some positive cases and negative cases 
> to look that if these cases could be parse into SQL statement which could be 
> executed successfully.
> 
> 
> Thanks,
> 
> Jianguo Tian
> 
>

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[Jianguo Tian]

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

(Updated Sept. 23, 2016, 7:31 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 (updated)
-

  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/TestRunInitialSQL.java PRE-CREATION 

Diff: https://reviews.apache.org/r/51695/diff/


Testing
---

TestRunInitialSQL.java is a JUnit test class which can test parseInitFile() 
method of HiveConnection.java. I test some positive cases and negative cases to 
look that if these cases could be parse into SQL statement which could be 
executed successfully.


Thanks,

Jianguo Tian

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[Jianguo Tian]

> On Sept. 23, 2016, 2:52 a.m., cheng xu wrote:
> > jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java, line 74
> > 
> >
> > You can change the expected variable as an array.And use the following 
> > to check equality:
> >   assertTrue(ArrayUtils.isEquals())

As method parseInitFile() return a List type, it's difficult to check equality 
with assertTrue(ArrayUtils.isEquals()). Besides, it looks that initializing 
expected as an array won't work, I have tried it.


- Jianguo


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


On Sept. 22, 2016, 6:49 a.m., Jianguo Tian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51695/
> ---
> 
> (Updated Sept. 22, 2016, 6:49 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/TestRunInitialSQL.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51695/diff/
> 
> 
> Testing
> ---
> 
> TestRunInitialSQL.java is a JUnit test class which can test parseInitFile() 
> method of HiveConnection.java. I test some positive cases and negative cases 
> to look that if these cases could be parse into SQL statement which could be 
> executed successfully.
> 
> 
> Thanks,
> 
> Jianguo Tian
> 
>

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[cheng xu]

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




jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java (line 18)


Break line here please.



jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java (line 53)


This should a comment, right?
e.g. //The cases above are some positive cases which could be executed. 
Here are some negative cases as below



jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java (line 67)


Can we do this one time setup in @BeforeClass method?



jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java (line 74)


You can change the expected variable as an array.And use the following to 
check equality:
  assertTrue(ArrayUtils.isEquals())



jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java (line 81)


Can we do this one time clean up in @AfterClass method?


- cheng xu


On Sept. 22, 2016, 2:49 p.m., Jianguo Tian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51695/
> ---
> 
> (Updated Sept. 22, 2016, 2:49 p.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/TestRunInitialSQL.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51695/diff/
> 
> 
> Testing
> ---
> 
> TestRunInitialSQL.java is a JUnit test class which can test parseInitFile() 
> method of HiveConnection.java. I test some positive cases and negative cases 
> to look that if these cases could be parse into SQL statement which could be 
> executed successfully.
> 
> 
> Thanks,
> 
> Jianguo Tian
> 
>

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[Jianguo Tian]

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

(Updated Sept. 22, 2016, 6:49 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 (updated)
-

  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/TestRunInitialSQL.java PRE-CREATION 

Diff: https://reviews.apache.org/r/51695/diff/


Testing
---

TestRunInitialSQL.java is a JUnit test class which can test parseInitFile() 
method of HiveConnection.java. I test some positive cases and negative cases to 
look that if these cases could be parse into SQL statement which could be 
executed successfully.


Thanks,

Jianguo Tian

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[Jianguo Tian]

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

(Updated Sept. 22, 2016, 6:48 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 (updated)
---

TestRunInitialSQL.java is a JUnit test class which can test parseInitFile() 
method of HiveConnection.java. I test some positive cases and negative cases to 
look that if these cases could be parse into SQL statement which could be 
executed successfully.


Thanks,

Jianguo Tian

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[Jianguo Tian]

> On Sept. 13, 2016, 2:52 a.m., Vihang Karajgaonkar wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java, line 211
> > 
> >
> > Will it work when we redirect the beeline output to a file?
> 
> Jianguo Tian wrote:
> I have write one Test class which will execute building JDBC connection 
> and initial SQL procedure, then I used "java -Djava.ext.dirs=/* Test > *.txt" 
> to redirect its output. All the content we can see in ".txt" file is initial 
> SQL result, namely output of System.out.pint. It means that we can redirect 
> output to a file.

And I used another way to verify my conclusion: I packaged my project into a 
jar with maven tool, then I write a test class which play the part of JDBC 
client(refer to 
https://cwiki.apache.org/confluence/display/Hive/HiveServer2+Clients, sample 
code of JDBC section). After runnig this test class, the output if initial SQL 
can be seen on the console.


- Jianguo


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


On Sept. 21, 2016, 3: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, 3: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
> 
>

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[cheng xu]

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


No need for "this."



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 250)


LOG.error



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 251)


Please throw SQLException.



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 256)


public method?



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 289)


private method?



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 295)


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



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 747)


Do we need this method? It's private.



jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 48)


Two space indents please.



jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 53)


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



jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 55)


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



jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 75)


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

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[Jianguo Tian]

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

(Updated Sept. 21, 2016, 3: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 (updated)
-

  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

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[Jianguo Tian]

> On Sept. 13, 2016, 2:52 a.m., Vihang Karajgaonkar wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java, line 211
> > 
> >
> > Will it work when we redirect the beeline output to a file?

I have write one Test class which will execute building JDBC connection and 
initial SQL procedure, then I used "java -Djava.ext.dirs=/* Test > *.txt" to 
redirect its output. All the content we can see in ".txt" file is initial SQL 
result, namely output of System.out.pint. It means that we can redirect output 
to a file.


- Jianguo


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


On Sept. 13, 2016, 2:36 a.m., Jianguo Tian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51695/
> ---
> 
> (Updated Sept. 13, 2016, 2:36 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 
>   jdbc/src/test/resources/init.sql 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
> 
>

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[Jianguo Tian]

> On Sept. 13, 2016, 2:52 a.m., Vihang Karajgaonkar wrote:
> > Can you please tell us how is this different than -i or --init option in 
> > the Beeline?

HIVE-5867 includes two parts of job, one is for Beeline, the other is for JDBC. 
And they both aim to support executing an initial SQL file, but Beeline has 
already support this function, whereas JDBC doesn't. So what I'm doing now is 
complete this function of JDBC.
The pattern of transmitting initial SQl file is diffrent, when build JDBC 
connection like 
"jdbc:hive2://localhost:1/default;initScript=/home/user1/scripts/init.sql", 
but Beeline is "-i". So we need to separate them into two parts, although they 
look like in almost same logic.


- Jianguo


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


On Sept. 13, 2016, 2:36 a.m., Jianguo Tian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51695/
> ---
> 
> (Updated Sept. 13, 2016, 2:36 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 
>   jdbc/src/test/resources/init.sql 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
> 
>

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[cheng xu]

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




jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 134)


Can you move the getter and setter after setupLoginTimeout method?



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 162)


Add a new line before this line.



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 183)


We should support it in embeded mode as well.



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 202)


remove "this."



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 252)


Use log



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 260)


Will this cover the following case?

show tables; show
tables;



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 273)


Please throw the original exception.



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 275)


if (br != null){
  br.close();
}



jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 1)


Please add license header.



jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 16)


Why do you need mock?



jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 18)


Can you add some negative cases?
e.g. 
# show tables; show
tables



jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 24)


L24-L29 is test case related and we should not add them in the setup method.



jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 37)


Please remove L37-38 which is debug used only.



jdbc/src/test/resources/init.sql (line 1)


Please use echo to create some tmp file and remove them after test case 
completed since we need to add many more cases like negative cases.


- cheng xu


On Sept. 13, 2016, 10:36 a.m., Jianguo Tian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51695/
> ---
> 
> (Updated Sept. 13, 2016, 10:36 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 
>   jdbc/src/test/resources/init.sql 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
> 
>

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[Vihang Karajgaonkar]

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



Can you please tell us how is this different than -i or --init option in the 
Beeline?


jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (lines 134 - 139)


Do we need these methods to be public? Looks like they are being used only 
withing this class. setInitFile may not be needed at all since initFile comes 
from sessConfMap.



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 211)


Will it work when we redirect the beeline output to a file?



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 252)


Does this print when we redirect the output to the file? Since this is a 
debug message I would suggest either using LOG.debug or System.err.println. 
Many users redirect the output of the command to a file. They might not want to 
see debug messages in the output file.


- Vihang Karajgaonkar


On Sept. 13, 2016, 2:36 a.m., Jianguo Tian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51695/
> ---
> 
> (Updated Sept. 13, 2016, 2:36 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 
>   jdbc/src/test/resources/init.sql 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
> 
>

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[Jianguo Tian]

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

(Updated Sept. 13, 2016, 2:36 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 (updated)
-

  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 
  jdbc/src/test/resources/init.sql PRE-CREATION 

Diff: https://reviews.apache.org/r/51695/diff/


Testing (updated)
---

TestInitSQL.java is JUnit test class which will test method initSql() in 
HiveConnection.java.


Thanks,

Jianguo Tian

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[Jianguo Tian]

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

(Updated Sept. 13, 2016, 2:07 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 (updated)
-

  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 
  jdbc/src/test/resources/init.sql PRE-CREATION 

Diff: https://reviews.apache.org/r/51695/diff/


Testing
---

TestHiveDriver.java is JUnit test class which will print the rusult of 
executing initial SQL(init.sql locate in src/test/resources).


Thanks,

Jianguo Tian

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[cheng xu]

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




jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 128)


Extra Space



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 150)


Extra Space



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 160)


Extra Space



jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 116)


2 space indent from L116 to L166



jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 127)


Will this handle the following case?
{noformat}
show
 tables;
{noformat}



jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 136)


Can we move it to the finally block?



jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 147)


LOG.error



jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 154)


Remove this line.



jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 156)


Extra space



jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 159)


Extra space



jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 161)


Extra space



jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 167)


No ident needed here.



jdbc/src/test/org/apache/hive/jdbc/TestHiveDriver.java (line 14)


Can this test pass without starting any server?



jdbc/src/test/org/apache/hive/jdbc/TestHiveDriver.java (line 19)


Extra space.



jdbc/src/test/resources/init.sql (line 3)


Remove this line since it's commented out.


- cheng xu


On Sept. 8, 2016, 8:52 a.m., Jianguo Tian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51695/
> ---
> 
> (Updated Sept. 8, 2016, 8:52 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/HiveDriver.java 
> a349f8bdad09f526f66a71768ecd2ca019671167 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 
> 3161566994d6c6e01de9d88a6e87295684619ffa 
>   jdbc/src/test/org/apache/hive/jdbc/TestHiveDriver.java PRE-CREATION 
>   jdbc/src/test/resources/init.sql PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51695/diff/
> 
> 
> Testing
> ---
> 
> TestHiveDriver.java is JUnit test class which will print the rusult of 
> executing initial SQL(init.sql locate in src/test/resources).
> 
> 
> Thanks,
> 
> Jianguo Tian
> 
>

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[Jianguo Tian]

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

(Updated Sept. 8, 2016, 12:52 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 (updated)
-

  jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java 
ad96a6466dd1aadab71fc261f55be4639dcbe2bf 
  jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 
a349f8bdad09f526f66a71768ecd2ca019671167 
  jdbc/src/java/org/apache/hive/jdbc/Utils.java 
3161566994d6c6e01de9d88a6e87295684619ffa 
  jdbc/src/test/org/apache/hive/jdbc/TestHiveDriver.java PRE-CREATION 
  jdbc/src/test/resources/init.sql PRE-CREATION 

Diff: https://reviews.apache.org/r/51695/diff/


Testing
---

TestHiveDriver.java is JUnit test class which will print the rusult of 
executing initial SQL(init.sql locate in src/test/resources).


Thanks,

Jianguo Tian

Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script

[Jianguo Tian]

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

(Updated Sept. 8, 2016, 12:51 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
-


Diff: https://reviews.apache.org/r/51695/diff/


Testing
---

TestHiveDriver.java is JUnit test class which will print the rusult of 
executing initial SQL(init.sql locate in src/test/resources).


File Attachments (updated)


HIVE-5867.1.patch
  
https://reviews.apache.org/media/uploaded/files/2016/09/08/72ebe7f1-4a30-489c-9b6c-8d5e79fdf500__HIVE-5867.1.patch


Thanks,

Jianguo Tian