zabetak commented on code in PR #3934:
URL: https://github.com/apache/hive/pull/3934#discussion_r1073456835
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -3122,7 +3117,19 @@ Seems much cleaner if each stmt is identified as a
particular HiveOperation (whi
}
return lockComponents;
}
-
+
+ private static LockType getLockTypeFromStorageHandler(WriteEntity output,
Table t) {
+ final HiveStorageHandler storageHandler =
Preconditions.checkNotNull(t.getStorageHandler(),
+ "Non-native tables must have an instance of storage handler.");
+ LockType lockType = storageHandler.getLockType(output);
+ if (null == LockType.findByValue(lockType.getValue())) {
Review Comment:
What's the benefit of doing `findByValue`? Isn't `if(null == lockType)`
already sufficient?
##########
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:
Changing the lock means that we are changing the tests. Why is it necessary?
--
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
