hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thejas Nair" <the...@hortonworks.com>
Subject Re: Review Request 23744: HIVE-7451 : pass function name in create/drop function to authorization api
Date Tue, 22 Jul 2014 19:50:49 GMT


> On July 22, 2014, 5:48 p.m., Jason Dere wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java,
line 16
> > <https://reviews.apache.org/r/23744/diff/1/?file=636932#file636932line16>
> >
> >     No changes relevant to patch - whitespace/imports removed. I guess it's not
so bad since this seems to be the only such file, I would make more of a stink if there were
lots of files like this in the patch.

For some reason I was getting build errors while testing because of the extra semi-colon here.
I don't remember exactly if the issue was only when I was trying to debug using eclipse.


> On July 22, 2014, 5:48 p.m., Jason Dere wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/FunctionSemanticAnalyzer.java, line
180
> > <https://reviews.apache.org/r/23744/diff/1/?file=636933#file636933line180>
> >
> >     Temp functions don't actually have an associated database, might be more appropriate
to set null DB here?
> >     
> >     Default DB used for temp functions in the WriteEntity created in line 174, just
enable us to check that user has admin privileges for creating temp functions.

Changing the temp function use case to not lookup the default db. So this database variable
is going to be null in case of temporary functions.


> On July 22, 2014, 5:48 p.m., Jason Dere wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java,
line 134
> > <https://reviews.apache.org/r/23744/diff/1/?file=636935#file636935line134>
> >
> >     Should database name (for metastore functions only, not really applicable for
temp functions) be included here as well)?

Yes, I think it makes sense to include the dbname if it is not null.


> On July 22, 2014, 5:48 p.m., Jason Dere wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAuthorizationValidator.java,
line 111
> > <https://reviews.apache.org/r/23744/diff/1/?file=636939#file636939line111>
> >
> >     If we ever support execute privileges for UDFS then for that case we would likely
want to check the metastore for execute privileges here. Would there be a way to have both
kinds of privilege checking behavior here?

We would also need to make changes in metastore to support function execute privileges. We
can make changes here as well at that time. As this part is implementation specific, we can
change it when the feature is added.


- Thejas


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


On July 21, 2014, 5:33 p.m., Thejas Nair wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23744/
> -----------------------------------------------------------
> 
> (Updated July 21, 2014, 5:33 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-7451
>     https://issues.apache.org/jira/browse/HIVE-7451
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/HIVE-7451
> 
> 
> Diffs
> -----
> 
>   contrib/src/test/results/clientnegative/case_with_row_sequence.q.out db564ff 
>   contrib/src/test/results/clientnegative/invalid_row_sequence.q.out 89646a2 
>   contrib/src/test/results/clientnegative/udtf_explode2.q.out 87dc534 
>   contrib/src/test/results/clientpositive/dboutput.q.out 909ae2e 
>   contrib/src/test/results/clientpositive/lateral_view_explode2.q.out 4b849fa 
>   contrib/src/test/results/clientpositive/udaf_example_avg.q.out 3786078 
>   contrib/src/test/results/clientpositive/udaf_example_group_concat.q.out 83b4802 
>   contrib/src/test/results/clientpositive/udaf_example_max.q.out b68ec61 
>   contrib/src/test/results/clientpositive/udaf_example_max_n.q.out 62632e3 
>   contrib/src/test/results/clientpositive/udaf_example_min.q.out ec3a134 
>   contrib/src/test/results/clientpositive/udaf_example_min_n.q.out 2e802e0 
>   contrib/src/test/results/clientpositive/udf_example_add.q.out 4510ba4 
>   contrib/src/test/results/clientpositive/udf_example_arraymapstruct.q.out 1e3bca4 
>   contrib/src/test/results/clientpositive/udf_example_format.q.out 83e508a 
>   contrib/src/test/results/clientpositive/udf_row_sequence.q.out 3b58cb5 
>   contrib/src/test/results/clientpositive/udtf_explode2.q.out 47512c3 
>   contrib/src/test/results/clientpositive/udtf_output_on_close.q.out 4ce0481 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/authorization/TestJdbcWithSQLAuthorization.java
3618185 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java c89f90c 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 40ec4e5 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/Entity.java 2a38aad 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java 26836b6 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java 37b1669

>   ql/src/java/org/apache/hadoop/hive/ql/parse/FunctionSemanticAnalyzer.java e64ef76 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java
604c39d 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java
8cdff5b 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/GrantPrivAuthUtils.java
1ac6cab 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLAuthorizationUtils.java
6b635ce 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAccessController.java
932b980 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAuthorizationValidator.java
c472cef 
>   ql/src/test/results/clientnegative/authorization_addjar.q.out 68c3c60 
>   ql/src/test/results/clientnegative/authorization_addpartition.q.out a14080a 
>   ql/src/test/results/clientnegative/authorization_alter_db_owner.q.out 928e9f5 
>   ql/src/test/results/clientnegative/authorization_alter_db_owner_default.q.out d4a617e

>   ql/src/test/results/clientnegative/authorization_compile.q.out cf5e4d1 
>   ql/src/test/results/clientnegative/authorization_create_func1.q.out 8863e91 
>   ql/src/test/results/clientnegative/authorization_create_func2.q.out 8863e91 
>   ql/src/test/results/clientnegative/authorization_create_macro1.q.out e4d410c 
>   ql/src/test/results/clientnegative/authorization_createview.q.out 3d0d191 
>   ql/src/test/results/clientnegative/authorization_ctas.q.out c9d0130 
>   ql/src/test/results/clientnegative/authorization_deletejar.q.out 71b11fd 
>   ql/src/test/results/clientnegative/authorization_desc_table_nosel.q.out 4583f56 
>   ql/src/test/results/clientnegative/authorization_dfs.q.out e95f563 
>   ql/src/test/results/clientnegative/authorization_drop_db_cascade.q.out 0bf82fc 
>   ql/src/test/results/clientnegative/authorization_drop_db_empty.q.out 93a3f1c 
>   ql/src/test/results/clientnegative/authorization_droppartition.q.out 3efabfe 
>   ql/src/test/results/clientnegative/authorization_fail_8.q.out 10dd71b 
>   ql/src/test/results/clientnegative/authorization_grant_table_allpriv.q.out ab4fd1c

>   ql/src/test/results/clientnegative/authorization_grant_table_fail1.q.out 0975a9c 
>   ql/src/test/results/clientnegative/authorization_grant_table_fail_nogrant.q.out 8e3d71c

>   ql/src/test/results/clientnegative/authorization_insert_noinspriv.q.out 332d8a4 
>   ql/src/test/results/clientnegative/authorization_insert_noselectpriv.q.out 1423e75

>   ql/src/test/results/clientnegative/authorization_insertoverwrite_nodel.q.out 458e65b

>   ql/src/test/results/clientnegative/authorization_not_owner_alter_tab_rename.q.out 39642e3

>   ql/src/test/results/clientnegative/authorization_not_owner_alter_tab_serdeprop.q.out
96df5a7 
>   ql/src/test/results/clientnegative/authorization_not_owner_drop_tab.q.out bf22e89 
>   ql/src/test/results/clientnegative/authorization_not_owner_drop_view.q.out acdc0f3

>   ql/src/test/results/clientnegative/authorization_priv_current_role_neg.q.out 16927fd

>   ql/src/test/results/clientnegative/authorization_rolehierarchy_privs.q.out 0dcb653

>   ql/src/test/results/clientnegative/authorization_select.q.out 7854200 
>   ql/src/test/results/clientnegative/authorization_select_view.q.out 9f1e07e 
>   ql/src/test/results/clientnegative/authorization_show_parts_nosel.q.out 306fe2e 
>   ql/src/test/results/clientnegative/authorization_truncate.q.out 3f19aa9 
>   ql/src/test/results/clientnegative/authorize_create_tbl.q.out 64ebd8b 
>   ql/src/test/results/clientnegative/create_function_nonexistent_class.q.out fcd5ce7

>   ql/src/test/results/clientnegative/create_function_nonudf_class.q.out 26565be 
>   ql/src/test/results/clientnegative/create_udaf_failure.q.out 433ec44 
>   ql/src/test/results/clientnegative/create_unknown_genericudf.q.out 1a2956f 
>   ql/src/test/results/clientnegative/create_unknown_udf_udaf.q.out 4263be9 
>   ql/src/test/results/clientnegative/drop_native_udf.q.out 81b1793 
>   ql/src/test/results/clientnegative/temp_table_authorize_create_tbl.q.out 64ebd8b 
>   ql/src/test/results/clientnegative/udf_function_does_not_implement_udf.q.out 0bf56a4

>   ql/src/test/results/clientnegative/udf_local_resource.q.out 9e6b09b 
>   ql/src/test/results/clientnegative/udf_nonexistent_resource.q.out 3843428 
>   ql/src/test/results/clientnegative/udf_test_error.q.out 3146652 
>   ql/src/test/results/clientnegative/udf_test_error_reduce.q.out c83c503 
>   ql/src/test/results/clientpositive/authorization_admin_almighty2.q.out 1c8c3e3 
>   ql/src/test/results/clientpositive/authorization_create_func1.q.out 45f93ba 
>   ql/src/test/results/clientpositive/autogen_colalias.q.out fa5a7b6 
>   ql/src/test/results/clientpositive/compile_processor.q.out e86e0f3 
>   ql/src/test/results/clientpositive/create_func1.q.out 62ca263 
>   ql/src/test/results/clientpositive/create_genericudaf.q.out 1641e01 
>   ql/src/test/results/clientpositive/create_genericudf.q.out f012951 
>   ql/src/test/results/clientpositive/create_udaf.q.out e48c25f 
>   ql/src/test/results/clientpositive/create_view.q.out e193a4f 
>   ql/src/test/results/clientpositive/drop_udf.q.out c60f431 
>   ql/src/test/results/clientpositive/ptf_register_tblfn.q.out 3b810ec 
>   ql/src/test/results/clientpositive/ptf_streaming.q.out 04ceedf 
>   ql/src/test/results/clientpositive/udaf_sum_list.q.out 51708b3 
>   ql/src/test/results/clientpositive/udf_compare_java_string.q.out e522e51 
>   ql/src/test/results/clientpositive/udf_context_aware.q.out 2e214c5 
>   ql/src/test/results/clientpositive/udf_logic_java_boolean.q.out f48c8b2 
>   ql/src/test/results/clientpositive/udf_testlength.q.out 28d96fa 
>   ql/src/test/results/clientpositive/udf_testlength2.q.out 4d2c407 
>   ql/src/test/results/clientpositive/udf_using.q.out b29e899 
>   ql/src/test/results/clientpositive/windowing_udaf2.q.out 4a4b6cf 
> 
> Diff: https://reviews.apache.org/r/23744/diff/
> 
> 
> Testing
> -------
> 
> Tests updated.
> 
> 
> Thanks,
> 
> Thejas Nair
> 
>


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