RussellSpitzer commented on a change in pull request #3059:
URL: https://github.com/apache/iceberg/pull/3059#discussion_r751679514
##########
File path: core/src/main/java/org/apache/iceberg/DataFiles.java
##########
@@ -218,7 +219,9 @@ public Builder withFormat(FileFormat newFormat) {
}
public Builder withPartition(StructLike newPartition) {
- this.partitionData = copyPartitionData(spec, newPartition,
partitionData);
+ if (isPartitioned) {
+ this.partitionData = copyPartitionData(spec, newPartition,
partitionData);
+ }
Review comment:
I would rather we fix this in one of the other ways we discussed. This
ends up feeling a little strange to me since you can call "withPartition" and
the result is ignored. I don't like when functions can take arguments but then
not doing anything with them.
##########
File path: core/src/main/java/org/apache/iceberg/DataFiles.java
##########
@@ -218,7 +219,9 @@ public Builder withFormat(FileFormat newFormat) {
}
public Builder withPartition(StructLike newPartition) {
- this.partitionData = copyPartitionData(spec, newPartition,
partitionData);
+ if (isPartitioned) {
+ this.partitionData = copyPartitionData(spec, newPartition,
partitionData);
+ }
Review comment:
I would rather we fix this in one of the other ways we discussed. This
ends up feeling a little strange to me since you can call "withPartition" and
the arg is ignored. I don't like when functions can take arguments but then not
doing anything with them.
--
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]
