----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66979/#review202679 -----------------------------------------------------------
Overall looks good. Would be great if we could have a junit test to confirm the owner value is same as the one being set from the alter table command to catch regressions. ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java Lines 217 (patched) <https://reviews.apache.org/r/66979/#comment284644> do we need to check for null for tbl.getOwnerType()? ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java Lines 428 (patched) <https://reviews.apache.org/r/66979/#comment284647> can ownerType be null? - Vihang Karajgaonkar On May 7, 2018, 2:36 a.m., Sergio Pena wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66979/ > ----------------------------------------------------------- > > (Updated May 7, 2018, 2:36 a.m.) > > > Review request for hive and Vihang Karajgaonkar. > > > Bugs: HIVE-19374 > https://issues.apache.org/jira/browse/HIVE-19374 > > > Repository: hive-git > > > Description > ------- > > This patch implements the new ALTER SET ... SET OWNER command and calls the > HMS api calls to change the owner of the table. > > The command syntax is: > > ALTER TABLE <table> SET OWNER { USER <user_name>|GROUP <group_name>|ROLE > > <role_name> } > > Currently, Hive sets the owner of a table to the user who created that table. > With this command, we will be able to change it to another user, group of > role (as ALTER DATABASE does). > > The changes are: > - HiveParser.g which adds the new syntax > - HiveOperation.java which adds the new ALTERTABLE_OWNER operation > - Table.java which gets/sets the owner type > - SemanticAnalyzer.java which returns the DDLSemanticAnalyzer if an > ALTERTABLE_OWNER operation is detected > - DDLSemanticAnalyzer.java which analyzes the ALTERTABLE_OWNER Operation > - AlterTableDesc.java uses by the DDL semantic analyzer to change the new > owner information > - MetaDataFormatUtils which displays the owner type when the DESCRIBE command > is called > - JsonMetaDataFormatted which is another implementation to display the owner > type in Json format > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java > 3141a7e981eb35a9fbc7f367f38f8ad420f1f928 > ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java > 879b4224494c3a9adb0713f319e586db4865fb17 > > ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java > cd70eee26c06ee6476964508c54c2bb10b167530 > > ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java > af283e693b5a0fc68e35221b2005fcf1910bdb8e > ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java > defb8becdb5d767ae71d5c962afac43f0c068c3c > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g > a837d67b9615ca1ee359c7aa26f79b6f2504dd99 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java > 820046388adbc65664ae36b08aaba72943ccb6af > ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableDesc.java > a767796a949da3c23ebe6d8c78b995c8638ebfef > ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java > cd4c206a89f1bc1a6195b0f1f39d3c4b462dc027 > ql/src/test/queries/clientpositive/alter_table_set_owner.q PRE-CREATION > ql/src/test/results/clientpositive/alter_table_set_owner.q.out PRE-CREATION > > > Diff: https://reviews.apache.org/r/66979/diff/1/ > > > Testing > ------- > > Waiting for HiveQA > > - alter_table_set_owner.q which verifies that the new command works. Describe > is not tested because the .q tests files mask the owner information. > - the describe command verified manually in my local hive environment > > > Thanks, > > Sergio Pena > >
