[GitHub] [iceberg] openinx commented on a change in pull request #2310: Core: change writers constructor to use StructLike

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

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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org