Copilot commented on code in PR #15175:
URL: https://github.com/apache/iceberg/pull/15175#discussion_r2739578213
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/RewritePositionDeleteFilesSparkAction.java:
##########
@@ -377,4 +378,9 @@ private String jobDesc(
table.name());
}
}
+
+ private String getPartitionDescription(StructLike partition) {
+ Preconditions.checkArgument(partition instanceof StructProjection);
Review Comment:
The precondition check lacks an error message explaining why the partition
must be a StructProjection. Add a descriptive message like 'Partition must be a
StructProjection to retrieve description'.
```suggestion
Preconditions.checkArgument(
partition instanceof StructProjection,
"Partition must be a StructProjection to retrieve description");
```
##########
core/src/main/java/org/apache/iceberg/util/PartitionUtil.java:
##########
@@ -105,9 +106,12 @@ private PartitionUtil() {}
// adapts the provided partition data to match the table partition type
public static StructLike coercePartition(
Types.StructType partitionType, PartitionSpec spec, StructLike
partition) {
- StructProjection projection =
- StructProjection.createAllowMissing(spec.partitionType(),
partitionType);
- projection.wrap(partition);
+ StructProjection projection =
StructProjection.createAllowMissing(spec.partitionType(), partitionType);
+ if (partition instanceof PartitionData) {
+ projection.wrap(partition, partition.toString());
+ } else {
+ return projection.wrap(partition);
+ }
return projection;
Review Comment:
The if-branch calls `wrap()` but doesn't return the projection, while the
else-branch returns immediately. This creates inconsistent behavior where the
if-branch falls through to return the projection after wrapping, but the
else-branch returns the result of `wrap()` directly. Either both branches
should return directly, or the if-branch should also use `return
projection.wrap(...)`.
```suggestion
return projection.wrap(partition, partition.toString());
} else {
return projection.wrap(partition);
}
```
--
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]
