You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Sergio Pena via Review Board <no...@reviews.apache.org> on 2018/05/07 02:36:57 UTC
Review Request 66979: HIVE-19374: Parse and process ALTER TABLE SET
OWNER command syntax
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66979/
-----------------------------------------------------------
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
Re: Review Request 66979: HIVE-19374: Parse and process ALTER TABLE
SET OWNER command syntax
Posted by Vihang Karajgaonkar via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request 66979: HIVE-19374: Parse and process ALTER TABLE
SET OWNER command syntax
Posted by Vihang Karajgaonkar via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66979/#review202771
-----------------------------------------------------------
Fix it, then Ship it!
LGTM. Thanks for the changes.
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/metadata/TestAlterTableMetadata.java
Lines 1 (patched)
<https://reviews.apache.org/r/66979/#comment284765>
Apache header missing
- Vihang Karajgaonkar
On May 8, 2018, 11:57 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66979/
> -----------------------------------------------------------
>
> (Updated May 8, 2018, 11:57 p.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
> -----
>
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/metadata/TestAlterTableMetadata.java PRE-CREATION
> 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
>
>
> Diff: https://reviews.apache.org/r/66979/diff/2/
>
>
> 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
>
>
Re: Review Request 66979: HIVE-19374: Parse and process ALTER TABLE
SET OWNER command syntax
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66979/
-----------------------------------------------------------
(Updated May 8, 2018, 11:57 p.m.)
Review request for hive and Vihang Karajgaonkar.
Changes
-------
- Check if ownerType is null on JsonMetaDataFormatted and MetaDataFormatUtils
- Write a test case that verifies the owner and owner type are changed on HMS with the alter table set owner command
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 (updated)
-----
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/metadata/TestAlterTableMetadata.java PRE-CREATION
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
Diff: https://reviews.apache.org/r/66979/diff/2/
Changes: https://reviews.apache.org/r/66979/diff/1-2/
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