tanmayrauth commented on code in PR #1627:
URL: https://github.com/apache/iceberg-go/pull/1627#discussion_r3706544005
##########
manifest.go:
##########
@@ -1594,6 +1598,19 @@ func NewManifestListWriterV3(out io.Writer, snapshotId,
sequenceNumber, firstRow
})
}
+func advanceRowID(firstRowID, existingRows, addedRows int64) (int64, error) {
+ if existingRows < 0 || addedRows < 0 {
Review Comment:
-1 isn't invalid here — it's the sentinel for a v1 manifest with unset row
counts (toFile at 263-273; see the comment at snapshot_producers.go:170). Since
append defaults to fastAppend (manifest-merge is off by default) and passes
inherited manifests through untouched into AddManifests, an inherited v1 data
manifest with unset counts hits this branch and fails the whole append with
"row counts must be non-negative: existing=-1 added=-1" — misleading, since
they're missing, not negative. Rejecting is fine (old code did += -2 and
corrupted the cursor), but handle the -1 case on purpose: recompute the count,
or bail with a message that names the real cause like "manifest %q has unknown
row counts, can't assign row-lineage IDs".
--
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]
