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