You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Dong Chen <do...@intel.com> on 2014/07/21 15:53:00 UTC
Review Request 23738: HIVE-5160: HS2 should support .hiverc
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23738/
-----------------------------------------------------------
Review request for hive.
Repository: hive-git
Description
-------
HIVE-5160: HS2 should support .hiverc
Diffs
-----
common/src/java/org/apache/hadoop/hive/common/cli/HiveFileProcessor.java PRE-CREATION
common/src/java/org/apache/hadoop/hive/common/cli/IHiveFileProcessor.java PRE-CREATION
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 8bff2a9
conf/hive-default.xml.template 4944dfc
service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 6a7ee7a
service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java e79b129
service/src/java/org/apache/hive/service/cli/session/SessionManager.java 6650c05
service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java PRE-CREATION
Diff: https://reviews.apache.org/r/23738/diff/
Testing
-------
UT passed.
Thanks,
Dong Chen
Re: Review Request 23738: HIVE-5160: HS2 should support .hiverc
Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23738/#review48439
-----------------------------------------------------------
Ship it!
Thanks Dong!
- Szehon Ho
On July 22, 2014, 8:24 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23738/
> -----------------------------------------------------------
>
> (Updated July 22, 2014, 8:24 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-5160: HS2 should support .hiverc
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/common/cli/HiveFileProcessor.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/common/cli/IHiveFileProcessor.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 593c566
> conf/hive-default.xml.template 653f5cc
> service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java a5c8e9b
> service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 7a3286d
> service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java e79b129
> service/src/java/org/apache/hive/service/cli/session/SessionManager.java 6650c05
> service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/23738/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 23738: HIVE-5160: HS2 should support .hiverc
Posted by Lefty Leverenz <le...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23738/#review48405
-----------------------------------------------------------
Ship it!
Ship It!
- Lefty Leverenz
On July 22, 2014, 8:24 a.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23738/
> -----------------------------------------------------------
>
> (Updated July 22, 2014, 8:24 a.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-5160: HS2 should support .hiverc
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/common/cli/HiveFileProcessor.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/common/cli/IHiveFileProcessor.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 593c566
> conf/hive-default.xml.template 653f5cc
> service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java a5c8e9b
> service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 7a3286d
> service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java e79b129
> service/src/java/org/apache/hive/service/cli/session/SessionManager.java 6650c05
> service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/23738/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 23738: HIVE-5160: HS2 should support .hiverc
Posted by Dong Chen <do...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23738/
-----------------------------------------------------------
(Updated July 22, 2014, 8:24 a.m.)
Review request for hive.
Changes
-------
An updated patch (HIVE-5160.1.patch) based on review comments
Repository: hive-git
Description
-------
HIVE-5160: HS2 should support .hiverc
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/common/cli/HiveFileProcessor.java PRE-CREATION
common/src/java/org/apache/hadoop/hive/common/cli/IHiveFileProcessor.java PRE-CREATION
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 593c566
conf/hive-default.xml.template 653f5cc
service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java a5c8e9b
service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 7a3286d
service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java e79b129
service/src/java/org/apache/hive/service/cli/session/SessionManager.java 6650c05
service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java PRE-CREATION
Diff: https://reviews.apache.org/r/23738/diff/
Testing
-------
UT passed.
Thanks,
Dong Chen
Re: Review Request 23738: HIVE-5160: HS2 should support .hiverc
Posted by Lefty Leverenz <le...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23738/#review48313
-----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/common/cli/IHiveFileProcessor.java
<https://reviews.apache.org/r/23738/#comment84755>
Nitpick: "an executable Hive file" (an, H)
In general, "Hive" should have an initial capital unless it's a command, parameter name, etc. (You could make the change in all your comments so the Javadocs look good.)
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/23738/#comment84762>
Typo in parameter name: global, not globel.
Also, this patch is based on the old HiveConf.java but the new one (HIVE-6037) includes each parameter's description from the template file. For example, the new HiveConf.java has this for hive.autogen.columnalias.prefix.label (which follows your new parameter):
HIVE_AUTOGEN_COLUMNALIAS_PREFIX_LABEL("hive.autogen.columnalias.prefix.label", "_c",
"String used as a prefix when auto generating column alias.\n" +
"By default the prefix label will be appended with a column position number to form the column alias. \n" +
"Auto generation would happen if an aggregate function is used in a select clause without an explicit alias."),
conf/hive-default.xml.template
<https://reviews.apache.org/r/23738/#comment84767>
Again, typo in parameter name (global, not globel).
Nitpick: "... then HS2 will search the file" should be "will search for the file" -- or did you mean HS2 will search in the file?
service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
<https://reviews.apache.org/r/23738/#comment84771>
Nitpick: HiveServer2 (all one word in CamelCase)
- Lefty Leverenz
On July 21, 2014, 1:53 p.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23738/
> -----------------------------------------------------------
>
> (Updated July 21, 2014, 1:53 p.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-5160: HS2 should support .hiverc
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/common/cli/HiveFileProcessor.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/common/cli/IHiveFileProcessor.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 8bff2a9
> conf/hive-default.xml.template 4944dfc
> service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 6a7ee7a
> service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java e79b129
> service/src/java/org/apache/hive/service/cli/session/SessionManager.java 6650c05
> service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/23738/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>
Re: Review Request 23738: HIVE-5160: HS2 should support .hiverc
Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23738/#review48317
-----------------------------------------------------------
Hi Dong, can we make sure it is running as the connecting user, rather than the hiveserver2 user? I know it is a global .rc file and thus same commands, but I think it should be the connecting user for both security and auditing. For example, in the comment below, I think we should use proxySession to execute the statement if security is enabled.
Everything else looks great.
service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
<https://reviews.apache.org/r/23738/#comment84789>
Is it possible to rework the order , so that we still use proxy Session?
service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java
<https://reviews.apache.org/r/23738/#comment84788>
Small issue, can we fix the spelling for password?
- Szehon Ho
On July 21, 2014, 1:53 p.m., Dong Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23738/
> -----------------------------------------------------------
>
> (Updated July 21, 2014, 1:53 p.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-5160: HS2 should support .hiverc
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/common/cli/HiveFileProcessor.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/common/cli/IHiveFileProcessor.java PRE-CREATION
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 8bff2a9
> conf/hive-default.xml.template 4944dfc
> service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 6a7ee7a
> service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java e79b129
> service/src/java/org/apache/hive/service/cli/session/SessionManager.java 6650c05
> service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/23738/diff/
>
>
> Testing
> -------
>
> UT passed.
>
>
> Thanks,
>
> Dong Chen
>
>