[ 
https://issues.apache.org/jira/browse/HIVE-26055?focusedWorklogId=774950&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-774950
 ]

ASF GitHub Bot logged work on HIVE-26055:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 26/May/22 08:56
            Start Date: 26/May/22 08:56
    Worklog Time Spent: 10m 
      Work Description: kgyrtkirk commented on code in PR #3247:
URL: https://github.com/apache/hive/pull/3247#discussion_r882462556


##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLAuthorizationUtils.java:
##########
@@ -271,7 +271,16 @@ 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 {
+          if(!metastoreClient.tableExists(hivePrivObject.getDbname(), 
hivePrivObject.getObjectName())) {

Review Comment:
   I think this method should throw an exception if the table doesn't exists - 
you can't own non-exiting items; that's misleading...
   
   * this is a private method - so it could only be called from this class
   * the `metastoreClient` is passed as an argument to this method - so I don't 
think that should be a problem...



##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/rename/AbstractAlterTableRenameAnalyzer.java:
##########
@@ -49,6 +55,13 @@ protected void analyzeCommand(TableName tableName, 
Map<String, String> partition
       setAcidDdlDesc(desc);
     }
     addInputsOutputsAlterTable(tableName, null, desc, desc.getType(), false);
+//    inputs.add(new ReadEntity(table));
+    outputs.clear();

Review Comment:
   I see - but this will make it more diverse...what do you think about the 
following:
   * split the `addInputsOutputsAlterTable` into 2 methods which are taking 
care of inputs/outputs
   * you could add back `addInputsOutputsAlterTable` which is calling these 2 
for backward compatibility
   * and call these 2 methods with old/new args from here?
   



##########
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java:
##########
@@ -3612,8 +3612,8 @@ private void testRenameTable(boolean blocking) throws 
Exception {
       txnMgr2.acquireLocks(driver2.getPlan(), ctx, null, false);
       locks = getLocks();
 
-      ShowLocksResponseElement checkLock = checkLock(LockType.EXCLUSIVE,
-        LockState.WAITING, "default", "tab_acid", null, locks);
+      ShowLocksResponseElement checkLock = checkLock(LockType.SHARED_READ,

Review Comment:
   this is the most serious issue - without addressing this I'm against this 
change





Issue Time Tracking
-------------------

    Worklog Id:     (was: 774950)
    Time Spent: 1h 10m  (was: 1h)

> 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: 1h 10m
>  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)

Reply via email to