[
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)
