[GitHub] [hive] difin commented on a diff in pull request #3606: HIVE-26496: Populating bucketId in the constructor instead of parse m…

[GitBox <gi...@apache.org>]

difin commented on code in PR #3606:
URL: https://github.com/apache/hive/pull/3606#discussion_r975377018


##########
ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcSplit.java:
##########
@@ -103,7 +103,7 @@ public OrcSplit(Path path, Object fileId, long offset, long length, String[] hos
     this.isOriginal = isOriginal;
     this.hasBase = hasBase;
     this.rootDir = rootDir;
-    int bucketId = AcidUtils.parseBucketId(path);
+    bucketId = AcidUtils.parseBucketId(path);

Review Comment:
   I didn't find explanation in history why bucket id was set in parse method. Regarding making bucketId final: bucketId is a private class member and cannot be set outside of OrcSplit. Before my change it was set using getPath() method from its parent class. This method uses 'fs' class member from the parent class which is package-private and unaccessible from OrcSplit. So, bucketId is set using path which cannot be modified and it can have only one value. Therefore in my opinion bucketId can be set final and it is safe to do. In other words, if bucketId is set in constructor or in parse method, it can be assigned only same unique value which is based on the path and the path can't be changed at runtime.



-- 
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-unsubscribe@hive.apache.org

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