openinx commented on a change in pull request #2310:
URL: https://github.com/apache/iceberg/pull/2310#discussion_r590943618
##########
File path: core/src/main/java/org/apache/iceberg/io/BaseTaskWriter.java
##########
@@ -222,18 +221,18 @@ public String toString() {
private abstract class BaseRollingWriter<W extends Closeable> implements
Closeable {
private static final int ROWS_DIVISOR = 1000;
- private final PartitionKey partitionKey;
+ private final StructLike partitionKey;
private EncryptedOutputFile currentFile = null;
private W currentWriter = null;
private long currentRows = 0;
- private BaseRollingWriter(PartitionKey partitionKey) {
+ private BaseRollingWriter(StructLike partitionKey) {
this.partitionKey = partitionKey;
openCurrent();
}
- abstract W newWriter(EncryptedOutputFile file, PartitionKey key);
+ abstract W newWriter(EncryptedOutputFile file, StructLike key);
Review comment:
As we are refactoring the type from `PartitionKey` to generic
`StructLike`, the argument `key` is not enough to express the meaning of
partition values. I will suggest to refactor the argument name from `key` to
`partitionData` or `partitionKey`, so that we could understand its meaning at
the first glance. This should be applied to the following code also.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
