You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hive.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2022/05/11 07:24:00 UTC
[jira] [Work logged] (HIVE-26055) Fix the HivePrivilegesObjects for Alter table rename command
[ https://issues.apache.org/jira/browse/HIVE-26055?focusedWorklogId=768862&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-768862 ]
ASF GitHub Bot logged work on HIVE-26055:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 11/May/22 07:23
Start Date: 11/May/22 07:23
Worklog Time Spent: 10m
Work Description: kgyrtkirk commented on code in PR #3247:
URL: https://github.com/apache/hive/pull/3247#discussion_r869945626
##########
itests/util/src/main/java/org/apache/hadoop/hive/ql/hooks/VerifyOutputTableLocationSchemeIsFileHook.java:
##########
@@ -24,10 +24,10 @@ public class VerifyOutputTableLocationSchemeIsFileHook implements ExecuteWithHoo
@Override
public void run(HookContext hookContext) {
- for (WriteEntity output : hookContext.getOutputs()) {
- if (output.getType() == WriteEntity.Type.TABLE) {
- String scheme = output.getTable().getDataLocation().toUri().getScheme();
- Assert.assertTrue(output.getTable().getTableName() + " has a location which has a " +
+ for (ReadEntity input : hookContext.getInputs()) {
Review Comment:
the logic here seem to have changed ; and its not aligned anymore with the name of the class
is this change intended? maybe you wanted to add a new hook which check for the input?
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLAuthorizationUtils.java:
##########
@@ -269,7 +269,19 @@ private static boolean isOwner(IMetaStoreClient metastoreClient, String userName
thriftTableObj = metastoreClient.getTable(hivePrivObject.getDbname(),
hivePrivObject.getObjectName());
} catch (Exception e) {
- throwGetObjErr(e, hivePrivObject);
+ boolean isTableExists = true;
+ try {
+ isTableExists = metastoreClient.tableExists(hivePrivObject.getDbname(), hivePrivObject.getObjectName());
+ } catch (TException ex){
+ throwGetObjErr(ex, hivePrivObject);
+ }
+ if(!isTableExists){
+ // Do not throw any exception when table object is not present.
+ // return true since the current user the owner of new table.
+ return true;
Review Comment:
I think instead of returning `true` here for non-existent tables; the "call-site" should check if the table already exists and make a decision based on that
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/rename/AbstractAlterTableRenameAnalyzer.java:
##########
@@ -48,7 +54,12 @@ protected void analyzeCommand(TableName tableName, Map<String, String> partition
if (AcidUtils.isTransactionalTable(table)) {
setAcidDdlDesc(desc);
}
- addInputsOutputsAlterTable(tableName, null, desc, desc.getType(), false);
Review Comment:
I think we are throwing out a lot of logic by not calling this method - and that might be the cause of the sheer volume of failures/test output changes
##########
ql/src/test/results/clientnegative/alter_table_wrong_db.q.out:
##########
@@ -18,8 +18,4 @@ POSTHOOK: query: create table rename1(a int)
POSTHOOK: type: CREATETABLE
POSTHOOK: Output: bad_rename1@rename1
POSTHOOK: Output: database:bad_rename1
-PREHOOK: query: alter table bad_rename1.rename1 rename to bad_db_notexists.rename1
-PREHOOK: type: ALTERTABLE_RENAME
-PREHOOK: Input: bad_rename1@rename1
-PREHOOK: Output: bad_rename1@rename1
-FAILED: Execution Error, return code 40013 from org.apache.hadoop.hive.ql.ddl.DDLTask. Unable to alter table. Unable to change partition or table. Object database hive.bad_db_notexists does not exist. Check metastore logs for detailed stack.
+FAILED: SemanticException [Error 10072]: Database does not exist: bad_db_notexists
Review Comment:
negative test fails with a different error
##########
ql/src/test/results/clientnegative/alter_view_failure8.q.out:
##########
@@ -6,4 +6,15 @@ POSTHOOK: query: CREATE TABLE invites (foo INT, bar STRING) PARTITIONED BY (ds S
POSTHOOK: type: CREATETABLE
POSTHOOK: Output: database:default
POSTHOOK: Output: default@invites
-FAILED: SemanticException [Error 10132]: To alter a base table you need to use the ALTER TABLE command.
+PREHOOK: query: ALTER VIEW invites RENAME TO invites2
+PREHOOK: type: ALTERVIEW_RENAME
+PREHOOK: Input: default@invites
+PREHOOK: Output: database:default
+PREHOOK: Output: default@invites2
+POSTHOOK: query: ALTER VIEW invites RENAME TO invites2
+POSTHOOK: type: ALTERVIEW_RENAME
+POSTHOOK: Input: default@invites
+POSTHOOK: Output: database:default
+POSTHOOK: Output: default@invites2
+FAILED: AssertionError java.lang.AssertionError: Client Execution was expected to fail, but succeeded with error code 0 for fname=alter_view_failure8.q
Review Comment:
negative test is failing
##########
ql/src/test/results/clientnegative/alter_non_native.q.out:
##########
@@ -8,4 +8,15 @@ STORED BY 'org.apache.hadoop.hive.ql.metadata.DefaultStorageHandler'
POSTHOOK: type: CREATETABLE
POSTHOOK: Output: database:default
POSTHOOK: Output: default@non_native1
-FAILED: SemanticException [Error 10134]: ALTER TABLE can only be used for [ADDPROPS, DROPPROPS, ADDCOLS] to a non-native table non_native1
+PREHOOK: query: ALTER TABLE non_native1 RENAME TO new_non_native
+PREHOOK: type: ALTERTABLE_RENAME
+PREHOOK: Input: default@non_native1
+PREHOOK: Output: database:default
+PREHOOK: Output: default@new_non_native
+POSTHOOK: query: ALTER TABLE non_native1 RENAME TO new_non_native
+POSTHOOK: type: ALTERTABLE_RENAME
+POSTHOOK: Input: default@non_native1
+POSTHOOK: Output: database:default
+POSTHOOK: Output: default@new_non_native
+FAILED: AssertionError java.lang.AssertionError: Client Execution was expected to fail, but succeeded with error code 0 for fname=alter_non_native.q
Review Comment:
negative test is failing
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLAuthorizationUtils.java:
##########
@@ -269,7 +269,19 @@ private static boolean isOwner(IMetaStoreClient metastoreClient, String userName
thriftTableObj = metastoreClient.getTable(hivePrivObject.getDbname(),
hivePrivObject.getObjectName());
} catch (Exception e) {
- throwGetObjErr(e, hivePrivObject);
+ boolean isTableExists = true;
+ try {
+ isTableExists = metastoreClient.tableExists(hivePrivObject.getDbname(), hivePrivObject.getObjectName());
Review Comment:
I think you don't need the `isTableExists` boolean ; you could just:
```
if(!metastoreClient.tableExists(hivePrivObject.getDbname(), hivePrivObject.getObjectName())) {
return true;
}
```
Issue Time Tracking
-------------------
Worklog Id: (was: 768862)
Time Spent: 20m (was: 10m)
> Fix the HivePrivilegesObjects for Alter table rename command
> ------------------------------------------------------------
>
> Key: HIVE-26055
> URL: https://issues.apache.org/jira/browse/HIVE-26055
> Project: Hive
> Issue Type: Bug
> Components: HiveServer2, Security
> Reporter: Sai Hemanth Gantasala
> Assignee: Sai Hemanth Gantasala
> Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Fix the HivePrivilegeObjects for Alter table rename query in a way that it includes source table information in the output objects and destination table information in the input objects.
>
--
This message was sent by Atlassian Jira
(v8.20.7#820007)