kasakrisz commented on code in PR #3934:
URL: https://github.com/apache/hive/pull/3934#discussion_r1073584822


##########
ql/src/test/org/apache/hadoop/hive/ql/metadata/StorageHandlerMock.java:
##########
@@ -70,7 +70,7 @@ public class StorageHandlerMock extends DefaultStorageHandler 
{
     if (writeEntity.getWriteType().equals(WriteEntity.WriteType.INSERT)) {
       return LockType.SHARED_READ;
     }
-    return LockType.SHARED_WRITE;
+    return LockType.EXCLUSIVE;

Review Comment:
   This mock is used in 2 test cases:
   * testLockingOnInsertIntoNonNativeTables
   * testLockingOnInsertOverwriteNonNativeTables
   
   Prior this patch this was ignored in case of insert overwrite and 
`EXCLUSIVE` was set in case of any type of non transactional table.
   After I altered the logic to use the lock type coming from the 
`storageHandler` the mock returned `SHARED_WRITE` and test 
`testLockingOnInsertOverwriteNonNativeTables` failed since it still expected 
`EXCLUSIVE`.
   
   I could alter the assertion in the test but since the default lock type 
specified by the storage handler is `EXCLUSIVE` I chose altering the mock.
   
https://github.com/apache/hive/blob/55471330426c2e0a52101c2e535a66f751be76ee/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveStorageHandler.java#L199-L201
   
   Maybe `super.getLockType` would be better here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to