You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2020/07/08 10:35:09 UTC

[GitHub] [hive] pvargacl opened a new pull request #1224: HIVE-23805: Provide ValidTxnList to AcidUtils::getAcidState

pvargacl opened a new pull request #1224:
URL: https://github.com/apache/hive/pull/1224


   
   


----------------------------------------------------------------
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.

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



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


[GitHub] [hive] pvargacl commented on a change in pull request #1224: HIVE-23805: Provide ValidTxnList to AcidUtils::getAcidState

Posted by GitBox <gi...@apache.org>.
pvargacl commented on a change in pull request #1224:
URL: https://github.com/apache/hive/pull/1224#discussion_r451446059



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
##########
@@ -1262,8 +1262,8 @@ public static boolean isAcid(FileSystem fileSystem, Path directory,
    * @throws IOException on filesystem errors
    */
   public static Directory getAcidState(FileSystem fileSystem, Path candidateDirectory, Configuration conf,
-      ValidWriteIdList writeIdList, Ref<Boolean> useFileIds, boolean ignoreEmptyFiles) throws IOException {
-    return getAcidState(fileSystem, candidateDirectory, conf, writeIdList, useFileIds, ignoreEmptyFiles, null);
+      ValidWriteIdList writeIdList, ValidTxnList validTxnList, Ref<Boolean> useFileIds, boolean ignoreEmptyFiles) throws IOException {
+    return getAcidState(fileSystem, candidateDirectory, conf, writeIdList, validTxnList, useFileIds, ignoreEmptyFiles, null);

Review comment:
       I will create a separate issue to change getAcidState to get just one parameter with builder pattern, because this is getting out of hand




----------------------------------------------------------------
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.

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



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


[GitHub] [hive] pvary commented on a change in pull request #1224: HIVE-23805: Provide ValidTxnList to AcidUtils::getAcidState

Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #1224:
URL: https://github.com/apache/hive/pull/1224#discussion_r451472066



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java
##########
@@ -730,7 +729,10 @@ public boolean validateInput(FileSystem fs, HiveConf conf,
           ? AcidOperationalProperties.parseString(txnProperties) : null;
 
       String value = conf.get(ValidWriteIdList.VALID_WRITEIDS_KEY);
-      writeIdList = value == null ? new ValidReaderWriteIdList() : new ValidReaderWriteIdList(value);
+      writeIdList = new ValidReaderWriteIdList(value);
+
+      value = conf.get(ValidTxnList.VALID_TXNS_KEY);
+      validTxnList = new ValidReadTxnList(value);

Review comment:
       Will this help, if we have multiple partitions with multiple files to parse the TxnList only once?




----------------------------------------------------------------
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.

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



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


[GitHub] [hive] github-actions[bot] commented on pull request #1224: HIVE-23805: Provide ValidTxnList to AcidUtils::getAcidState

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #1224:
URL: https://github.com/apache/hive/pull/1224#issuecomment-687952773


   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
   Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.


----------------------------------------------------------------
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.

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



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


[GitHub] [hive] pvary commented on a change in pull request #1224: HIVE-23805: Provide ValidTxnList to AcidUtils::getAcidState

Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #1224:
URL: https://github.com/apache/hive/pull/1224#discussion_r451470405



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
##########
@@ -1262,8 +1262,8 @@ public static boolean isAcid(FileSystem fileSystem, Path directory,
    * @throws IOException on filesystem errors
    */
   public static Directory getAcidState(FileSystem fileSystem, Path candidateDirectory, Configuration conf,
-      ValidWriteIdList writeIdList, Ref<Boolean> useFileIds, boolean ignoreEmptyFiles) throws IOException {
-    return getAcidState(fileSystem, candidateDirectory, conf, writeIdList, useFileIds, ignoreEmptyFiles, null);
+      ValidWriteIdList writeIdList, ValidTxnList validTxnList, Ref<Boolean> useFileIds, boolean ignoreEmptyFiles) throws IOException {
+    return getAcidState(fileSystem, candidateDirectory, conf, writeIdList, validTxnList, useFileIds, ignoreEmptyFiles, null);

Review comment:
       Maybe create a separate class for AcidState?




----------------------------------------------------------------
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.

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



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


[GitHub] [hive] github-actions[bot] closed pull request #1224: HIVE-23805: Provide ValidTxnList to AcidUtils::getAcidState

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #1224:
URL: https://github.com/apache/hive/pull/1224


   


----------------------------------------------------------------
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.

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



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