zabetak commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1522923087
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java:
##########
@@ -69,40 +80,48 @@ public AlterTableCompactOperation(DDLOperationContext
context, AlterTableCompact
compactionRequest.setNumberOfBuckets(desc.getNumberOfBuckets());
}
- InitiatorBase initiatorBase = new InitiatorBase();
- initiatorBase.setConf(context.getConf());
Review Comment:
Utility methods should not implicitly change the arguments especially things
like configurations cause this makes them harder to use and can easily lead to
bugs.
I believe we can modify the `CompactorUtil.initiateCompactionForPartition`
method to not modify the input configuration by creating the `ValidTxnList`
when needed (i.e., inside the `resolveValidWriteIds` method) unless there is a
specific reason that we need this to be done at the very beginning.
Assuming that it is really necessary to do the initialization of
`ValidTxnList` here then we could just propagate this object as it is to
subsequent methods by adding an additional param.
A final option would be to make a copy of the whole config object right
before modifying it but this would be costly and wasteful in terms of resources.
--
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]
