Re: [PR] fix(table): fail closed when a partition spec id is not registered (#1855) [iceberg-go]

["zeroshade (via GitHub)" <gi...@apache.org>]

zeroshade commented on code in PR #1856:
URL: https://github.com/apache/iceberg-go/pull/1856#discussion_r3832759093


##########
table/snapshot_producers.go:
##########
@@ -1002,7 +1013,12 @@ func (sp *snapshotProducer) writeAddedManifest(content iceberg.ManifestContent,
 }
 
 func (sp *snapshotProducer) writeAddedDeleteManifest(specID int, additions []deleteFileAddition) (_ iceberg.ManifestFile, retErr error) {
-	wr, path, counter, out, err := sp.newManifestWriter(sp.spec(specID), iceberg.WithManifestWriterContent(iceberg.ManifestContentDeletes))
+	spec, err := sp.spec(specID)

Review Comment:
   Blocking: this new error path can be reached after another manifest has already been persisted. `addedContentManifests` runs the data and delete producers in parallel, so a RowDelta containing valid data plus a delete file with an unregistered spec writes the data manifest, returns `ErrPartitionSpecNotFound` here, and then `errgroup.Wait` discards the successful manifest path. Since no snapshot is staged, that file is never cleaned up. A focused probe left `metadata/<uuid>-m1.avro`; an earlier valid group followed by an invalid group has the same problem. Please preflight all referenced spec IDs before creating any output, or remove all completed manifests on failure, and add a mixed valid/invalid test asserting that no manifest remains.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org