zeroshade commented on code in PR #1988:
URL: https://github.com/apache/iceberg-go/pull/1988#discussion_r3960485768
##########
table/metadata.go:
##########
@@ -2053,24 +2058,27 @@ func assignMissingPartitionFieldIDsFromMetadata(b
[]byte, metadata map[string]js
return nil, err
}
field["field-id"] = rawFieldID
+ normalizedLastPartitionID = max(normalizedLastPartitionID,
lastAssignedID)
}
- 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:
**nit** — Unknown-spec-key fidelity guard is itself unpinned by tests
The 'if len(missingFields) > 0' guard prevents the stale-counter-only path
from round-tripping specs through rawPartitionSpec, which would drop spec-level
keys the struct cannot represent. Replacing the condition with 'true' leaves
the entire table package green, so the preservation behaviour has no regression
test. TestAssignMissingPartitionFieldIDsNormalizesLegacyStaleCounter passes
through this path but only asserts last-partition-id, never the spec bytes.
Non-blocking; the guard is an improvement over pre-PR behaviour either way.
--
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]
