----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23744/#review48353 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java <https://reviews.apache.org/r/23744/#comment84900> 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. ql/src/java/org/apache/hadoop/hive/ql/parse/FunctionSemanticAnalyzer.java <https://reviews.apache.org/r/23744/#comment84905> So there will be 2 WriteEntities generated for CREATE FUNCTION - one on the DB object (to check admin privs), and one on the function being created. Can you explain the use case here? Also, looking at the way the FUNCTION Entity is validated in SQLStdHiveAuthorizationValidator.java, it simply checks for admin. Would we be able to just replace the older Database Entity check with the Function Entity? ql/src/java/org/apache/hadoop/hive/ql/parse/FunctionSemanticAnalyzer.java <https://reviews.apache.org/r/23744/#comment84904> 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. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java <https://reviews.apache.org/r/23744/#comment84903> Should database name (for metastore functions only, not really applicable for temp functions) be included here as well)? ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAuthorizationValidator.java <https://reviews.apache.org/r/23744/#comment84939> 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? - Jason Dere 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 > >
