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


##########
ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcSplit.java:
##########
@@ -384,7 +384,6 @@ public void parse(Configuration conf, Path rootPath) throws 
IOException {
         
OrcRawRecordMerger.TransactionMetaData.findWriteIDForSynthetcRowIDs(getPath(), 
rootPath, conf);
     writeId = tmd.syntheticWriteId;
     stmtId = tmd.statementId;
-    bucketId = AcidUtils.parseBucketId(getPath());

Review Comment:
   Yes but do we *always* pass through the constructor which sets the 
`bucketId`?
   
   The `*Split` classes are serializable which makes me a bit skeptical about 
which constructor is called at each point.   
   
   Moreover, `writeId` and `stmtId` are also set here and they seem to depend 
exclusively on `getPath()` which again makes me a bit suspicious why they were 
not set in constructor if was possible to do so.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to