Skip to content

Commit 83aa988

Browse files
fix: preserve snapshot ID width in UseRef (#661)
## Summary - keep `UseRef()` snapshot IDs as `int64_t` while resolving refs - add a regression test for a ref pointing at `INT32_MAX + 42` ## Root Cause `TableScanBuilder::UseRef()` copied `SnapshotRef::snapshot_id` into an `int32_t`, even though snapshot IDs are modeled as `int64_t` in refs, metadata lookup, and scan context. ## Testing - `uvx cmake --build build --target scan_test -j 8` - `./build/src/iceberg/test/scan_test --gtest_filter=*UseRefPreservesInt64SnapshotIds*` - `./build/src/iceberg/test/scan_test` Fixes #660
1 parent d690aaa commit 83aa988

2 files changed

Lines changed: 26 additions & 1 deletion

File tree

src/iceberg/table_scan.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ TableScanBuilder<ScanType>& TableScanBuilder<ScanType>::UseRef(const std::string
430430
auto iter = metadata_->refs.find(ref);
431431
ICEBERG_BUILDER_CHECK(iter != metadata_->refs.end(), "Cannot find ref {}", ref);
432432
ICEBERG_BUILDER_CHECK(iter->second != nullptr, "Ref {} is null", ref);
433-
int32_t snapshot_id = iter->second->snapshot_id;
433+
const int64_t snapshot_id = iter->second->snapshot_id;
434434
ICEBERG_BUILDER_ASSIGN_OR_RETURN(std::ignore, metadata_->SnapshotById(snapshot_id));
435435
context_.snapshot_id = snapshot_id;
436436

src/iceberg/test/table_scan_test.cc

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
* under the License.
1818
*/
1919

20+
#include <limits>
2021
#include <memory>
2122
#include <optional>
2223
#include <string>
@@ -205,6 +206,30 @@ TEST_P(TableScanTest, TableScanBuilderOptions) {
205206
EXPECT_EQ(snapshot->snapshot_id, 1000L);
206207
}
207208

209+
TEST_P(TableScanTest, UseRefPreservesInt64SnapshotIds) {
210+
constexpr int64_t kLargeSnapshotId =
211+
static_cast<int64_t>(std::numeric_limits<int32_t>::max()) + 42;
212+
table_metadata_->snapshots.push_back(std::make_shared<Snapshot>(
213+
Snapshot{.snapshot_id = kLargeSnapshotId,
214+
.parent_snapshot_id = table_metadata_->current_snapshot_id,
215+
.sequence_number = 2L,
216+
.timestamp_ms = TimePointMsFromUnixMs(1609459201000L),
217+
.manifest_list = "/tmp/metadata/snap-large-2-manifest-list.avro",
218+
.schema_id = schema_->schema_id()}));
219+
table_metadata_->refs["branch-with-large-snapshot-id"] = std::make_shared<SnapshotRef>(
220+
SnapshotRef{.snapshot_id = kLargeSnapshotId, .retention = SnapshotRef::Branch{}});
221+
222+
ICEBERG_UNWRAP_OR_FAIL(auto builder,
223+
DataTableScanBuilder::Make(table_metadata_, file_io_));
224+
builder->UseRef("branch-with-large-snapshot-id");
225+
ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build());
226+
227+
ASSERT_TRUE(scan->context().snapshot_id.has_value());
228+
EXPECT_EQ(scan->context().snapshot_id.value(), kLargeSnapshotId);
229+
ICEBERG_UNWRAP_OR_FAIL(auto snapshot, scan->snapshot());
230+
EXPECT_EQ(snapshot->snapshot_id, kLargeSnapshotId);
231+
}
232+
208233
TEST_P(TableScanTest, TableScanBuilderValidationErrors) {
209234
// Test negative min rows
210235
ICEBERG_UNWRAP_OR_FAIL(auto builder,

0 commit comments

Comments
 (0)