You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "laskoviymishka (via GitHub)" <gi...@apache.org> on 2026/06/30 21:05:57 UTC
[PR] fix(table): clarify v3 next-row-id overflow guard, harden tests [iceberg-go]
laskoviymishka opened a new pull request, #1348:
URL: https://github.com/apache/iceberg-go/pull/1348
Follow-up to #1054. cleanup the next-row-id overflow guard in validateAndUpdateRowLineage to check overflow before computing the wrapped sum (added-rows > MaxInt64 - next-row-id) and reword the error to say what actually happened ("adding N rows to next-row-id M overflows int64") rather than the misleading "regressed". Document the invariants the guard relies on: added-rows is non-nil/non-negative and next-row-id is non-negative at that point.
Tests: drop the redundant positive-advance case (covered by TestAddSnapshotV3UpdatesNextRowID), make the zero-added-rows test falsifiable by seeding a non-zero cursor first, and add the MaxInt64 accept boundary alongside the overflow reject.
--
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
Re: [PR] fix(table): clarify v3 next-row-id overflow guard, harden tests [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #1348:
URL: https://github.com/apache/iceberg-go/pull/1348
--
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
Re: [PR] fix(table): clarify v3 next-row-id overflow guard, harden tests [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #1348:
URL: https://github.com/apache/iceberg-go/pull/1348#discussion_r3521378581
##########
table/metadata_builder_internal_test.go:
##########
@@ -1702,13 +1703,61 @@ func TestAddSnapshotV3AcceptsFirstRowIDEqualToNextRowID(t *testing.T) {
require.Equal(t, int64(100), *builder.nextRowID)
}
-func TestAddSnapshotV3AcceptsPositiveAddedRows(t *testing.T) {
- // Positive added-rows should advance next-row-id.
+func TestAddSnapshotV3AcceptsZeroAddedRows(t *testing.T) {
+ // Asserts spec-permitted no-op behavior: a zero-added-rows snapshot
+ // (delete-only or metadata-only commit) leaves next-row-id unchanged. The
+ // cursor is first advanced to a non-zero value so the assertion is
+ // falsifiable — a regression that advanced the cursor on every commit would
+ // be caught. This is NOT coverage of the overflow guard — see
+ // TestAddSnapshotV3RejectsNextRowIDOverflow.
builder := builderWithoutChanges(3)
schemaID := 0
- firstRowID := int64(0)
- addedRows := int64(50)
+ appendFirstRowID := int64(0)
+ appendAddedRows := int64(50)
+ appendSnapshot := Snapshot{
+ SnapshotID: 1,
+ ParentSnapshotID: nil,
+ SequenceNumber: 0,
+ TimestampMs: builder.base.LastUpdatedMillis() + 1,
+ ManifestList: "/snap-1.avro",
+ Summary: &Summary{Operation: OpAppend},
+ SchemaID: &schemaID,
+ FirstRowID: &appendFirstRowID,
+ AddedRows: &appendAddedRows,
+ }
+ require.NoError(t, builder.AddSnapshot(&appendSnapshot))
+ require.Equal(t, int64(50), *builder.nextRowID)
+
+ deleteFirstRowID := int64(50)
+ deleteAddedRows := int64(0)
+ deleteSnapshot := Snapshot{
+ SnapshotID: 2,
+ ParentSnapshotID: &appendSnapshot.SnapshotID,
+ SequenceNumber: 1,
+ TimestampMs: builder.base.LastUpdatedMillis() + 2,
+ ManifestList: "/snap-2.avro",
+ Summary: &Summary{Operation: OpDelete},
+ SchemaID: &schemaID,
+ FirstRowID: &deleteFirstRowID,
+ AddedRows: &deleteAddedRows,
+ }
+ require.NoError(t, builder.AddSnapshot(&deleteSnapshot))
+ require.Equal(t, int64(50), *builder.nextRowID)
+}
+
+func TestAddSnapshotV3RejectsNextRowIDOverflow(t *testing.T) {
+ // next-row-id + added-rows must not overflow int64. Seed the cursor near
+ // the maximum so a positive added-rows tips it over. This is the only input
+ // that reaches the guard: negative added-rows is rejected earlier by
+ // Snapshot.ValidateRowLineage.
+ builder := builderWithoutChanges(3)
+ nearMax := int64(math.MaxInt64 - 10)
+ builder.nextRowID = &nearMax
+
+ schemaID := 0
+ firstRowID := nearMax
+ addedRows := int64(20)
Review Comment:
Nit (optional): this rejects at `MaxInt64 - 10 + 20` = `MaxInt64 + 10`. To pin the exact boundary against a future off-by-one, consider `addedRows := int64(11)` (sum == `MaxInt64 + 1`) for the reject case, plus a `MaxInt64 - 1` accept case.
--
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