----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23674/#review49498 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java <https://reviews.apache.org/r/23674/#comment86575> doesn't the default authorization mode support columns in show-grants ? It is there in ShowGrantDesc ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java <https://reviews.apache.org/r/23674/#comment86591> can you please add javadoc for this and getTableName, what it does is not obvious from function name. Maybe also change the argument name to dbTableName ? ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java <https://reviews.apache.org/r/23674/#comment86590> Since this is a common utility function, how about also throwing an error if child count is > 2 ? Maybe use Preconditions like HIVE-7561 does. ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java <https://reviews.apache.org/r/23674/#comment86592> how about using BaseSemanticAnalyzer. getQualifiedTableName and having this check for 'duplicate declaration' there ? ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java <https://reviews.apache.org/r/23674/#comment86707> Isn't it better to represent the columns as a set instead of list, as multiple columns with same name in this object does not make sense ? Same for other places in this patch where columns has been changed from a set to list. ql/src/test/results/clientpositive/temp_table_precedence.q.out <https://reviews.apache.org/r/23674/#comment86708> Some of the test q.out updates show that this is fixing issue for some cases being fixed in the jira. Are there any .q files that need to be updated ? For example, any .q files to be updated for describe/show partition/show tblproperties ? - Thejas Nair On Aug. 1, 2014, 1:55 a.m., Navis Ryu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23674/ > ----------------------------------------------------------- > > (Updated Aug. 1, 2014, 1:55 a.m.) > > > Review request for hive and Thejas Nair. > > > Bugs: HIVE-4064 > https://issues.apache.org/jira/browse/HIVE-4064 > > > Repository: hive-git > > > Description > ------- > > Hive doesn't consistently handle db qualified names across all HiveQL > statements. While some HiveQL statements such as SELECT support DB qualified > names, other such as CREATE INDEX doesn't. > > > Diffs > ----- > > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java > c91b15c > > itests/util/src/main/java/org/apache/hadoop/hive/ql/hooks/CheckColumnAccessHook.java > 14fc430 > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > b74868b > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java > 5a56ced > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > 4f186f4 > ql/src/java/org/apache/hadoop/hive/ql/Driver.java cba5cfa > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 40d910c > ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 4cf4522 > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java a7e50ad > ql/src/java/org/apache/hadoop/hive/ql/optimizer/IndexUtils.java ae87aac > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteGBUsingIndex.java > 11a6d07 > ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java > 22945e3 > ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnAccessInfo.java 939dc65 > ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java > 67a3aa7 > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g ab1188a > ql/src/java/org/apache/hadoop/hive/ql/parse/IndexUpdater.java 856ec2f > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 51838ae > > ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryImpl.java > 826bdf3 > ql/src/java/org/apache/hadoop/hive/ql/plan/AlterIndexDesc.java 0318e4b > ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableAlterPartDesc.java > cf67e16 > ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableSimpleDesc.java > 541675c > ql/src/java/org/apache/hadoop/hive/ql/plan/PrivilegeObjectDesc.java 9417220 > ql/src/java/org/apache/hadoop/hive/ql/plan/RenamePartitionDesc.java 1b5fb9e > ql/src/java/org/apache/hadoop/hive/ql/plan/ShowColumnsDesc.java fe6a91e > ql/src/java/org/apache/hadoop/hive/ql/plan/ShowGrantDesc.java aa88153 > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java > 5c94217 > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java > 9e9ef71 > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java > fbc0090 > ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java 98c2924 > ql/src/test/org/apache/hadoop/hive/ql/parse/TestQBCompact.java 5f32d5f > > ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/PrivilegesTestBase.java > 93901ec > > ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestHiveAuthorizationTaskFactory.java > ab0d80e > > ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV1.java > fd827ad > > ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV2.java > 9499986 > ql/src/test/results/clientnegative/alter_concatenate_indexed_table.q.out > 500d45d > ql/src/test/results/clientnegative/alter_view_failure6.q.out cfbaca8 > ql/src/test/results/clientnegative/merge_negative_1.q.out 95f6678 > ql/src/test/results/clientnegative/merge_negative_2.q.out b3422e1 > ql/src/test/results/clientnegative/show_columns3.q.out 09068b7 > ql/src/test/results/clientnegative/show_tableproperties1.q.out ca54088 > ql/src/test/results/clientnegative/temp_table_index.q.out 8ec5c0a > ql/src/test/results/clientpositive/drop_multi_partitions.q.out eae57f3 > ql/src/test/results/clientpositive/input3.q.out 547449c > ql/src/test/results/clientpositive/insert2_overwrite_partitions.q.out > 21bd257 > ql/src/test/results/clientpositive/show_create_table_db_table.q.out 0119471 > ql/src/test/results/clientpositive/show_tblproperties.q.out 80db5e4 > ql/src/test/results/clientpositive/temp_table_names.q.out 940684c > ql/src/test/results/clientpositive/temp_table_precedence.q.out 1075b2c > > Diff: https://reviews.apache.org/r/23674/diff/ > > > Testing > ------- > > > Thanks, > > Navis Ryu > >
