> On Aug. 5, 2014, 5:09 a.m., Thejas Nair wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java, line 607 > > <https://reviews.apache.org/r/23674/diff/2/?file=647687#file647687line607> > > > > doesn't the default authorization mode support columns in show-grants ? > > It is there in ShowGrantDesc > > > > Navis Ryu wrote: > I've moved columns in ShowGrantDesc to PrivilegeObjectDesc, which seemed > more neat, imho. Isn't it?
Yes, thats certainly better. > On Aug. 5, 2014, 5:09 a.m., Thejas Nair wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java, line > > 2110 > > <https://reviews.apache.org/r/23674/diff/2/?file=647694#file647694line2110> > > > > how about using BaseSemanticAnalyzer. getQualifiedTableName and having > > this check for 'duplicate declaration' there ? > > > > > > Navis Ryu wrote: > It's not TOK_TABNAME (which is TOK_FROM identifier? > (identifier|StringLiteral)?) and seemed not replaced with > getQualifiedTableName() Thanks for clarifying! > On Aug. 5, 2014, 5:09 a.m., Thejas Nair wrote: > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java, > > line 94 > > <https://reviews.apache.org/r/23674/diff/2/?file=647707#file647707line94> > > > > 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. > > Navis Ryu wrote: > HivePrivilegeObject compares columns by iteration. If columns is not > ordered somehow, it seemed not a valid comparison. I didn't have a idea to > compare two column sets, I just replaced it to a sorted list, which felt > easier that that. Any idea? Sounds fine. Maybe we can do a copy and sort as part of the constructor of this HivePrivilegeObject, instead of relying on the argument being sorted. That is likely to avoid potential bugs. But that can be done as part of separate jira. - Thejas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23674/#review49498 ----------------------------------------------------------- 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 > >
