mattfaltyn commented on code in PR #1988:
URL: https://github.com/apache/iceberg-go/pull/1988#discussion_r3927520149
##########
table/metadata.go:
##########
@@ -2016,11 +2016,13 @@ func assignMissingPartitionFieldIDsFromMetadata(b
[]byte, metadata map[string]js
}
lastAssignedID := iceberg.PartitionDataIDStart - 1
+ lastPartitionID := lastAssignedID
Review Comment:
Thanks for catching this. Fixed in ebf0a16 by separating the persisted
counter target from the 999 allocation cursor. Empty specs with sub-999
counters now retain the original bytes and catalog-visible value, while stale
counters with assigned fields still normalize correctly.
##########
table/metadata_preflight_test.go:
##########
@@ -85,6 +85,16 @@ func TestParseMetadataBytesAssignsMissingPartitionFieldIDs(t
*testing.T) {
}
}
+func TestParseMetadataBytesNormalizesStaleLastPartitionID(t *testing.T) {
+ data := strings.Replace(ExampleTableMetadataV2,
Review Comment:
Thanks for identifying the missing branches. Added coverage in ebf0a16 for
sub-999 counters with no assigned fields, counters above the greatest assigned
field ID, and stale counters combined with a missing field ID. The first two
cases also verify the unchanged-byte fast path.
##########
table/metadata.go:
##########
@@ -2055,18 +2057,20 @@ func assignMissingPartitionFieldIDsFromMetadata(b
[]byte, metadata map[string]js
field["field-id"] = rawFieldID
}
- if usesSpecList {
- rawSpecs, err := json.Marshal(specs)
- if err != nil {
- return nil, err
- }
- metadata["partition-specs"] = rawSpecs
- } else {
- rawFields, err := json.Marshal(specs[0].Fields)
- if err != nil {
- return nil, err
+ if len(missingFields) > 0 {
+ if usesSpecList {
Review Comment:
Thanks for flagging this. I confirmed the unknown partition-spec key loss
predates this change and left it unchanged to keep #1987 focused. The
stale-counter-only path introduced here continues to avoid partition-spec
re-marshalling.
--
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]
