rdblue commented on a change in pull request #2829:
URL: https://github.com/apache/iceberg/pull/2829#discussion_r705753743
##########
File path:
spark/src/main/java/org/apache/iceberg/spark/actions/BaseRewriteDataFilesSparkAction.java
##########
@@ -149,7 +168,13 @@ public RewriteDataFiles filter(Expression expression) {
try {
Map<StructLike, List<FileScanTask>> filesByPartition =
Streams.stream(fileScanTasks)
- .collect(Collectors.groupingBy(task -> task.file().partition()));
+ .collect(Collectors.groupingBy(task -> {
+ if (task.file().specId() == table.spec().specId()) {
+ return task.file().partition();
+ } else {
+ return EmptyStruct.get();
Review comment:
Okay, so all data where we're changing the partitioning goes into a
separate task. That makes sense to me. Probably worth a comment here to explain
it.
You may also want to change how this works slightly and use a
`StructLikeMap`. Otherwise, `EmptyStruct` looks a lot like structs for the
unpartitioned spec. It would or wouldn't be equal depending on the order of the
comparison, which probably isn't what you want.
Instead, I would keep a `StructLikeMap` for the current partitioning scheme
and then create a separate group using a `List` for the rest.
--
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]
