You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Alexander Pivovarov <ap...@gmail.com> on 2015/02/01 21:11:29 UTC
Review Request 30487: HIVE-9143 impl current_user() udf
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30487/
-----------------------------------------------------------
Review request for hive.
Bugs: HIVE-9143
https://issues.apache.org/jira/browse/HIVE-9143
Repository: hive-git
Description
-------
HIVE-9143 impl current_user() udf
Diffs
-----
ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 23d77ca4cc2e2a44b62f62ddbd4826df092bcfe8
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java PRE-CREATION
ql/src/test/queries/clientpositive/udf_current_user.q PRE-CREATION
ql/src/test/results/clientpositive/show_functions.q.out 36c8743a61c55a714352d358a5d9cc0deb4cef2c
ql/src/test/results/clientpositive/udf_current_user.q.out PRE-CREATION
shims/common/src/main/java/org/apache/hadoop/hive/shims/Utils.java c851dc2cb28876aef77811ead397429a2338cde4
Diff: https://reviews.apache.org/r/30487/diff/
Testing
-------
Thanks,
Alexander Pivovarov
Re: Review Request 30487: HIVE-9143 impl current_user() udf
Posted by Jason Dere <jd...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30487/#review70613
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java
<https://reviews.apache.org/r/30487/#comment115843>
In Thejas' review of HIVE-5472 (current_date/current_timestamp), he pointed out that the udftype can probably be set to deterministic because the value of current_user() should not change within the same query. Looks like we currently do not have a type to describe UDFs whose value is constant within the query but can change between queries, which this and current_date/current_timestamp fall under.
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java
<https://reviews.apache.org/r/30487/#comment115874>
Can you also add setters/getters to currentUser? Hive needs to serialize/deserialize the query plan, and there are 2 options here: javaXML (XMLEncoder) and kryo serialization. In the case of javaXML serialization appropriate getters/setters need to exist for non-transient fields that need to be preserved.
ql/src/test/queries/clientpositive/udf_current_user.q
<https://reviews.apache.org/r/30487/#comment115875>
Take a look at clientpositive/authorization_view_sqlstd.q, I think this has an example of being able to set the hive username to a fixed value during a test. Can you see if this works for testing current_user()?
- Jason Dere
On Feb. 1, 2015, 8:20 p.m., Alexander Pivovarov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30487/
> -----------------------------------------------------------
>
> (Updated Feb. 1, 2015, 8:20 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-9143
> https://issues.apache.org/jira/browse/HIVE-9143
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-9143 impl current_user() udf
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 23d77ca4cc2e2a44b62f62ddbd4826df092bcfe8
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java PRE-CREATION
> ql/src/test/queries/clientpositive/udf_current_user.q PRE-CREATION
> ql/src/test/results/clientpositive/show_functions.q.out 36c8743a61c55a714352d358a5d9cc0deb4cef2c
> ql/src/test/results/clientpositive/udf_current_user.q.out PRE-CREATION
> shims/common/src/main/java/org/apache/hadoop/hive/shims/Utils.java c851dc2cb28876aef77811ead397429a2338cde4
>
> Diff: https://reviews.apache.org/r/30487/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexander Pivovarov
>
>
Re: Review Request 30487: HIVE-9143 impl current_user() udf
Posted by Thejas Nair <th...@hortonworks.com>.
> On Feb. 3, 2015, 10:31 a.m., Jason Dere wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java, line 50
> > <https://reviews.apache.org/r/30487/diff/2-3/?file=843182#file843182line50>
> >
> > Thejas would probably know the answer to this - does getUserFromAuthenticator() still return a value even if SQL Std auth is not enabled?
Yes, that call works even if SQL std auth is not enabled. That api call is also used during creation of metastore objects for identifying the owner.
- Thejas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30487/#review70730
-----------------------------------------------------------
On Feb. 2, 2015, 10:42 p.m., Alexander Pivovarov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30487/
> -----------------------------------------------------------
>
> (Updated Feb. 2, 2015, 10:42 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-9143
> https://issues.apache.org/jira/browse/HIVE-9143
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-9143 impl current_user() udf
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java bfb4dc292b6b9f1ee342bb2e28b2afc722bb3167
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java PRE-CREATION
> ql/src/test/queries/clientpositive/udf_current_user.q PRE-CREATION
> ql/src/test/results/clientpositive/show_functions.q.out e21b54bd6c3b5f51eb45c733bbe838fd78abe641
> ql/src/test/results/clientpositive/udf_current_user.q.out PRE-CREATION
> shims/common/src/main/java/org/apache/hadoop/hive/shims/Utils.java c851dc2cb28876aef77811ead397429a2338cde4
>
> Diff: https://reviews.apache.org/r/30487/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexander Pivovarov
>
>
Re: Review Request 30487: HIVE-9143 impl current_user() udf
Posted by Alexander Pivovarov <ap...@gmail.com>.
> On Feb. 3, 2015, 10:31 a.m., Jason Dere wrote:
> >
I.
I tested the current_user() function for SQL Std auth. See below
1. I configured hive the following way. hive-site.xml
{code}
<property>
<name>hive.server2.enable.doAs</name>
<value>false</value>
</property>
<property>
<name>hive.security.authorization.manager</name>
<value>org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactory</value>
</property>
<property>
<name>hive.security.authorization.enabled</name>
<value>true</value>
</property>
<property>
<name>hive.security.authenticator.manager</name>
<value>org.apache.hadoop.hive.ql.security.SessionStateUserAuthenticator</value>
</property>
<property>
<name>hive.metastore.uris</name>
<value></value>
</property>
<property>
<name>hive.users.in.admin.role</name>
<value>apivovarov</value>
</property>
{code}
2. I started hiveserver2
3. I connected to hs2 as admin user "apivovarov"
4. I run "set role admin;"
5. I run "select current_user() from dual;". result: apivovarov
6. I have dual table in default db. I run "grant select on dual to role public;"
7. I connected to hs2 as regular user "alex"
8. I run "select current_user() from dual;". result: alex
So, current_user() works for SQL Standard Based Hive Authorization
II.
I also tested current_user() function for "apivovarov" and "alex" users using default hive settings (DefaultHiveAuthorization)
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30487/#review70730
-----------------------------------------------------------
On Feb. 2, 2015, 10:42 p.m., Alexander Pivovarov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30487/
> -----------------------------------------------------------
>
> (Updated Feb. 2, 2015, 10:42 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-9143
> https://issues.apache.org/jira/browse/HIVE-9143
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-9143 impl current_user() udf
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java bfb4dc292b6b9f1ee342bb2e28b2afc722bb3167
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java PRE-CREATION
> ql/src/test/queries/clientpositive/udf_current_user.q PRE-CREATION
> ql/src/test/results/clientpositive/show_functions.q.out e21b54bd6c3b5f51eb45c733bbe838fd78abe641
> ql/src/test/results/clientpositive/udf_current_user.q.out PRE-CREATION
> shims/common/src/main/java/org/apache/hadoop/hive/shims/Utils.java c851dc2cb28876aef77811ead397429a2338cde4
>
> Diff: https://reviews.apache.org/r/30487/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexander Pivovarov
>
>
Re: Review Request 30487: HIVE-9143 impl current_user() udf
Posted by Thejas Nair <th...@hortonworks.com>.
> On Feb. 3, 2015, 10:31 a.m., Jason Dere wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java, line 50
> > <https://reviews.apache.org/r/30487/diff/2-3/?file=843182#file843182line50>
> >
> > Thejas would probably know the answer to this - does getUserFromAuthenticator() still return a value even if SQL Std auth is not enabled?
>
> Thejas Nair wrote:
> Yes, that call works even if SQL std auth is not enabled. That api call is also used during creation of metastore objects for identifying the owner.
I will let Jason give the final +1, as I have only focussed on just this couple of lines of this patch.
- Thejas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30487/#review70730
-----------------------------------------------------------
On Feb. 2, 2015, 10:42 p.m., Alexander Pivovarov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30487/
> -----------------------------------------------------------
>
> (Updated Feb. 2, 2015, 10:42 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-9143
> https://issues.apache.org/jira/browse/HIVE-9143
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-9143 impl current_user() udf
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java bfb4dc292b6b9f1ee342bb2e28b2afc722bb3167
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java PRE-CREATION
> ql/src/test/queries/clientpositive/udf_current_user.q PRE-CREATION
> ql/src/test/results/clientpositive/show_functions.q.out e21b54bd6c3b5f51eb45c733bbe838fd78abe641
> ql/src/test/results/clientpositive/udf_current_user.q.out PRE-CREATION
> shims/common/src/main/java/org/apache/hadoop/hive/shims/Utils.java c851dc2cb28876aef77811ead397429a2338cde4
>
> Diff: https://reviews.apache.org/r/30487/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexander Pivovarov
>
>
Re: Review Request 30487: HIVE-9143 impl current_user() udf
Posted by Jason Dere <jd...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30487/#review70730
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java
<https://reviews.apache.org/r/30487/#comment116107>
Thejas would probably know the answer to this - does getUserFromAuthenticator() still return a value even if SQL Std auth is not enabled?
- Jason Dere
On Feb. 2, 2015, 10:42 p.m., Alexander Pivovarov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30487/
> -----------------------------------------------------------
>
> (Updated Feb. 2, 2015, 10:42 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-9143
> https://issues.apache.org/jira/browse/HIVE-9143
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-9143 impl current_user() udf
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java bfb4dc292b6b9f1ee342bb2e28b2afc722bb3167
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java PRE-CREATION
> ql/src/test/queries/clientpositive/udf_current_user.q PRE-CREATION
> ql/src/test/results/clientpositive/show_functions.q.out e21b54bd6c3b5f51eb45c733bbe838fd78abe641
> ql/src/test/results/clientpositive/udf_current_user.q.out PRE-CREATION
> shims/common/src/main/java/org/apache/hadoop/hive/shims/Utils.java c851dc2cb28876aef77811ead397429a2338cde4
>
> Diff: https://reviews.apache.org/r/30487/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexander Pivovarov
>
>
Re: Review Request 30487: HIVE-9143 impl current_user() udf
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30487/#review70812
-----------------------------------------------------------
ql/src/test/queries/clientpositive/udf_current_user.q
<https://reviews.apache.org/r/30487/#comment116252>
just a tip, you don't need to update the patch for this. The from clause is optional now. You can also do -
select current_user();
- Thejas Nair
On Feb. 2, 2015, 10:42 p.m., Alexander Pivovarov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30487/
> -----------------------------------------------------------
>
> (Updated Feb. 2, 2015, 10:42 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-9143
> https://issues.apache.org/jira/browse/HIVE-9143
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-9143 impl current_user() udf
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java bfb4dc292b6b9f1ee342bb2e28b2afc722bb3167
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java PRE-CREATION
> ql/src/test/queries/clientpositive/udf_current_user.q PRE-CREATION
> ql/src/test/results/clientpositive/show_functions.q.out e21b54bd6c3b5f51eb45c733bbe838fd78abe641
> ql/src/test/results/clientpositive/udf_current_user.q.out PRE-CREATION
> shims/common/src/main/java/org/apache/hadoop/hive/shims/Utils.java c851dc2cb28876aef77811ead397429a2338cde4
>
> Diff: https://reviews.apache.org/r/30487/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexander Pivovarov
>
>
Re: Review Request 30487: HIVE-9143 impl current_user() udf
Posted by Alexander Pivovarov <ap...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30487/
-----------------------------------------------------------
(Updated Feb. 2, 2015, 10:42 p.m.)
Review request for hive.
Changes
-------
fixed 4 issues found by Thejas
Bugs: HIVE-9143
https://issues.apache.org/jira/browse/HIVE-9143
Repository: hive-git
Description
-------
HIVE-9143 impl current_user() udf
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java bfb4dc292b6b9f1ee342bb2e28b2afc722bb3167
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java PRE-CREATION
ql/src/test/queries/clientpositive/udf_current_user.q PRE-CREATION
ql/src/test/results/clientpositive/show_functions.q.out e21b54bd6c3b5f51eb45c733bbe838fd78abe641
ql/src/test/results/clientpositive/udf_current_user.q.out PRE-CREATION
shims/common/src/main/java/org/apache/hadoop/hive/shims/Utils.java c851dc2cb28876aef77811ead397429a2338cde4
Diff: https://reviews.apache.org/r/30487/diff/
Testing
-------
Thanks,
Alexander Pivovarov
Re: Review Request 30487: HIVE-9143 impl current_user() udf
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30487/#review70638
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java
<https://reviews.apache.org/r/30487/#comment115922>
This will not work with HiveServer2 is configured with hive.server2.enable.doAs=false (the reccomended mode for sql standard authorization, apache ranger, sentry).
To make it work in that mode as well, you can use SessionState.getUserFromAuthenticator(). That method is not a public api, but since this is a builtin udf, we can use that api call. The user information from this called is used for authorization and ownership information in create-table etc.
- Thejas Nair
On Feb. 1, 2015, 8:20 p.m., Alexander Pivovarov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30487/
> -----------------------------------------------------------
>
> (Updated Feb. 1, 2015, 8:20 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-9143
> https://issues.apache.org/jira/browse/HIVE-9143
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-9143 impl current_user() udf
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 23d77ca4cc2e2a44b62f62ddbd4826df092bcfe8
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java PRE-CREATION
> ql/src/test/queries/clientpositive/udf_current_user.q PRE-CREATION
> ql/src/test/results/clientpositive/show_functions.q.out 36c8743a61c55a714352d358a5d9cc0deb4cef2c
> ql/src/test/results/clientpositive/udf_current_user.q.out PRE-CREATION
> shims/common/src/main/java/org/apache/hadoop/hive/shims/Utils.java c851dc2cb28876aef77811ead397429a2338cde4
>
> Diff: https://reviews.apache.org/r/30487/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexander Pivovarov
>
>
Re: Review Request 30487: HIVE-9143 impl current_user() udf
Posted by Alexander Pivovarov <ap...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30487/
-----------------------------------------------------------
(Updated Feb. 1, 2015, 8:20 p.m.)
Review request for hive.
Changes
-------
fixed show_functions.q.out
Bugs: HIVE-9143
https://issues.apache.org/jira/browse/HIVE-9143
Repository: hive-git
Description
-------
HIVE-9143 impl current_user() udf
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 23d77ca4cc2e2a44b62f62ddbd4826df092bcfe8
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java PRE-CREATION
ql/src/test/queries/clientpositive/udf_current_user.q PRE-CREATION
ql/src/test/results/clientpositive/show_functions.q.out 36c8743a61c55a714352d358a5d9cc0deb4cef2c
ql/src/test/results/clientpositive/udf_current_user.q.out PRE-CREATION
shims/common/src/main/java/org/apache/hadoop/hive/shims/Utils.java c851dc2cb28876aef77811ead397429a2338cde4
Diff: https://reviews.apache.org/r/30487/diff/
Testing
-------
Thanks,
Alexander Pivovarov