shanielh commented on code in PR #10737:
URL: https://github.com/apache/iceberg/pull/10737#discussion_r1703554438
##########
core/src/main/java/org/apache/iceberg/BaseUpdatePartitionSpec.java:
##########
@@ -62,15 +62,20 @@ class BaseUpdatePartitionSpec implements
UpdatePartitionSpec {
private int lastAssignedPartitionId;
BaseUpdatePartitionSpec(TableOperations ops) {
+ this(ops, ops.current().spec());
+ }
+
+ BaseUpdatePartitionSpec(TableOperations ops, PartitionSpec spec) {
this.ops = ops;
this.caseSensitive = true;
this.base = ops.current();
this.formatVersion = base.formatVersion();
- this.spec = base.spec();
+ this.spec = spec;
this.schema = spec.schema();
this.nameToField = indexSpecByName(spec);
this.transformToField = indexSpecByTransform(spec);
- this.lastAssignedPartitionId = base.lastAssignedPartitionId();
+ this.lastAssignedPartitionId =
Review Comment:
As the spec requires:
```
In v2, partition field IDs must be explicitly tracked for each partition
field. New IDs are assigned based on the last assigned partition ID in table
metadata.
In v1, partition field IDs were not tracked, but were assigned sequentially
starting at 1000 in the reference implementation.
```
Since we always evolve the latest spec, `base.lastAssignedPartitionId` would
work for both v1 and v2, but now when we're able to evolve from any spec (which
might be non-latest), it doesn't work and requires a branch in the code.
Reverting this would fail some of the new tests:
- `testReAddFieldUsingFromUnpartitionedSpec`
- 🔴 `formatVersion = 1`
- ✅ `formatVersion = 2`
- ✅ `formatVersion = 3`
- `testCommitFromSpec`
- 🔴 `formatVersion = 1`
- ✅ `formatVersion = 2`
- ✅ `formatVersion = 3`
I did modified the branch to condition on `formatVersion == 1` instead of
`formatVersion == 2` for better forward compatibility.
--
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]
