You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/03/10 02:12:14 UTC
[GitHub] [iceberg] chenjunjiedada opened a new pull request #2310: Core: change writers constructor to use StructLike
chenjunjiedada opened a new pull request #2310:
URL: https://github.com/apache/iceberg/pull/2310
This changes the writers' constructor to accept `StructLike` instead of `PartitionKey`, so that writers could create files using `PartitionData`.
----------------------------------------------------------------
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
[GitHub] [iceberg] chenjunjiedada commented on a change in pull request #2310: Core: change writers constructor to use StructLike
Posted by GitBox <gi...@apache.org>.
chenjunjiedada commented on a change in pull request #2310:
URL: https://github.com/apache/iceberg/pull/2310#discussion_r590978797
##########
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:
Make sense to me!
----------------------------------------------------------------
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
[GitHub] [iceberg] openinx merged pull request #2310: Core: change writers constructor to use StructLike
Posted by GitBox <gi...@apache.org>.
openinx merged pull request #2310:
URL: https://github.com/apache/iceberg/pull/2310
----------------------------------------------------------------
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
[GitHub] [iceberg] openinx commented on a change in pull request #2310: Core: change writers constructor to use StructLike
Posted by 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