Skip to content

Commit e516a75

Browse files
fix: invalidate table scan schema cache (#758)
## Summary - invalidate the cached scan snapshot schema whenever snapshot/ref/time-travel state changes - preserve requested column-stat names and re-resolve them during Build so stats IDs match the final snapshot schema - add regression coverage for IncludeColumnStats before UseSnapshot and non-main UseRef with an evolved schema ## Tests - cmake --build build --target scan_test - ctest --test-dir build -R scan_test --output-on-failure
1 parent 75bef3f commit e516a75

3 files changed

Lines changed: 81 additions & 12 deletions

File tree

src/iceberg/table_scan.cc

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -239,24 +239,16 @@ TableScanBuilder<ScanType>& TableScanBuilder<ScanType>::CaseSensitive(
239239
template <typename ScanType>
240240
TableScanBuilder<ScanType>& TableScanBuilder<ScanType>::IncludeColumnStats() {
241241
context_.return_column_stats = true;
242+
context_.columns_to_keep_stats.clear();
243+
requested_column_stats_.reset();
242244
return *this;
243245
}
244246

245247
template <typename ScanType>
246248
TableScanBuilder<ScanType>& TableScanBuilder<ScanType>::IncludeColumnStats(
247249
const std::vector<std::string>& requested_columns) {
248250
context_.return_column_stats = true;
249-
context_.columns_to_keep_stats.clear();
250-
context_.columns_to_keep_stats.reserve(requested_columns.size());
251-
252-
ICEBERG_BUILDER_ASSIGN_OR_RETURN(auto schema_ref, ResolveSnapshotSchema());
253-
const auto& schema = schema_ref.get();
254-
for (const auto& column_name : requested_columns) {
255-
ICEBERG_BUILDER_ASSIGN_OR_RETURN(auto field, schema->FindFieldByName(column_name));
256-
if (field.has_value()) {
257-
context_.columns_to_keep_stats.insert(field.value().get().field_id());
258-
}
259-
}
251+
requested_column_stats_ = requested_columns;
260252

261253
return *this;
262254
}
@@ -307,7 +299,6 @@ TableScanBuilder<ScanType>& TableScanBuilder<ScanType>::UseSnapshot(int64_t snap
307299
template <typename ScanType>
308300
TableScanBuilder<ScanType>& TableScanBuilder<ScanType>::UseRef(const std::string& ref) {
309301
if (ref == SnapshotRef::kMainBranch) {
310-
snapshot_schema_ = nullptr;
311302
context_.snapshot_id.reset();
312303
return *this;
313304
}
@@ -396,6 +387,27 @@ TableScanBuilder<ScanType>& TableScanBuilder<ScanType>::UseBranch(
396387
return *this;
397388
}
398389

390+
template <typename ScanType>
391+
Status TableScanBuilder<ScanType>::ResolveColumnStatsSelection() {
392+
if (!requested_column_stats_.has_value()) {
393+
return {};
394+
}
395+
396+
context_.columns_to_keep_stats.clear();
397+
context_.columns_to_keep_stats.reserve(requested_column_stats_->size());
398+
399+
ICEBERG_ASSIGN_OR_RAISE(auto schema_ref, ResolveSnapshotSchema());
400+
const auto& schema = schema_ref.get();
401+
for (const auto& column_name : *requested_column_stats_) {
402+
ICEBERG_ASSIGN_OR_RAISE(auto field, schema->FindFieldByName(column_name));
403+
if (field.has_value()) {
404+
context_.columns_to_keep_stats.insert(field.value().get().field_id());
405+
}
406+
}
407+
408+
return {};
409+
}
410+
399411
template <typename ScanType>
400412
Result<std::reference_wrapper<const std::shared_ptr<Schema>>>
401413
TableScanBuilder<ScanType>::ResolveSnapshotSchema() {
@@ -416,6 +428,7 @@ TableScanBuilder<ScanType>::ResolveSnapshotSchema() {
416428
template <typename ScanType>
417429
Result<std::unique_ptr<ScanType>> TableScanBuilder<ScanType>::Build() {
418430
ICEBERG_RETURN_UNEXPECTED(CheckErrors());
431+
ICEBERG_RETURN_UNEXPECTED(ResolveColumnStatsSelection());
419432
ICEBERG_RETURN_UNEXPECTED(context_.Validate());
420433

421434
ICEBERG_ASSIGN_OR_RAISE(auto schema, ResolveSnapshotSchema());

src/iceberg/table_scan.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,11 +392,13 @@ class ICEBERG_TEMPLATE_CLASS_EXPORT TableScanBuilder : public ErrorCollector {
392392

393393
// Return the schema bound to the specified snapshot.
394394
Result<std::reference_wrapper<const std::shared_ptr<Schema>>> ResolveSnapshotSchema();
395+
Status ResolveColumnStatsSelection();
395396

396397
std::shared_ptr<TableMetadata> metadata_;
397398
std::shared_ptr<FileIO> io_;
398399
internal::TableScanContext context_;
399400
std::shared_ptr<Schema> snapshot_schema_;
401+
std::optional<std::vector<std::string>> requested_column_stats_;
400402
};
401403

402404
/// \brief Represents a configured scan operation on a table.

src/iceberg/test/table_scan_test.cc

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,60 @@ TEST_P(TableScanTest, UseRefPreservesInt64SnapshotIds) {
231231
EXPECT_EQ(snapshot->snapshot_id, kLargeSnapshotId);
232232
}
233233

234+
TEST_P(TableScanTest, IncludeColumnStatsUsesFinalSnapshotSchema) {
235+
constexpr int64_t kBaseSnapshotId = 1000L;
236+
constexpr int64_t kEvolvedSnapshotId = 2000L;
237+
constexpr int32_t kBaseIdFieldId = 1;
238+
constexpr int32_t kEvolvedIdFieldId = 10;
239+
constexpr int32_t kEvolvedDataFieldId = 11;
240+
constexpr int32_t kEvolvedSchemaId = 1;
241+
242+
auto evolved_schema = std::make_shared<Schema>(
243+
std::vector<SchemaField>{
244+
SchemaField::MakeRequired(kEvolvedIdFieldId, "id", int32()),
245+
SchemaField::MakeRequired(kEvolvedDataFieldId, "data", string())},
246+
kEvolvedSchemaId);
247+
table_metadata_->schemas.push_back(evolved_schema);
248+
table_metadata_->last_column_id = kEvolvedDataFieldId;
249+
table_metadata_->snapshots.push_back(std::make_shared<Snapshot>(
250+
Snapshot{.snapshot_id = kEvolvedSnapshotId,
251+
.parent_snapshot_id = kBaseSnapshotId,
252+
.sequence_number = 2L,
253+
.timestamp_ms = TimePointMsFromUnixMs(1609459201000L),
254+
.manifest_list = "/tmp/metadata/snap-2000-2-manifest-list.avro",
255+
.schema_id = evolved_schema->schema_id()}));
256+
table_metadata_->refs["evolved-branch"] = std::make_shared<SnapshotRef>(
257+
SnapshotRef{.snapshot_id = kEvolvedSnapshotId, .retention = SnapshotRef::Branch{}});
258+
259+
{
260+
ICEBERG_UNWRAP_OR_FAIL(auto builder,
261+
DataTableScanBuilder::Make(table_metadata_, file_io_));
262+
builder->IncludeColumnStats({"id"}).UseSnapshot(kEvolvedSnapshotId);
263+
ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build());
264+
ICEBERG_UNWRAP_OR_FAIL(auto scan_schema, scan->schema());
265+
266+
EXPECT_EQ(scan_schema->schema_id(), evolved_schema->schema_id());
267+
const auto& stats_fields = scan->context().columns_to_keep_stats;
268+
EXPECT_EQ(stats_fields.size(), 1);
269+
EXPECT_TRUE(stats_fields.contains(kEvolvedIdFieldId));
270+
EXPECT_FALSE(stats_fields.contains(kBaseIdFieldId));
271+
}
272+
273+
{
274+
ICEBERG_UNWRAP_OR_FAIL(auto builder,
275+
DataTableScanBuilder::Make(table_metadata_, file_io_));
276+
builder->IncludeColumnStats({"id"}).UseRef("evolved-branch");
277+
ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build());
278+
ICEBERG_UNWRAP_OR_FAIL(auto scan_schema, scan->schema());
279+
280+
EXPECT_EQ(scan_schema->schema_id(), evolved_schema->schema_id());
281+
const auto& stats_fields = scan->context().columns_to_keep_stats;
282+
EXPECT_EQ(stats_fields.size(), 1);
283+
EXPECT_TRUE(stats_fields.contains(kEvolvedIdFieldId));
284+
EXPECT_FALSE(stats_fields.contains(kBaseIdFieldId));
285+
}
286+
}
287+
234288
TEST_P(TableScanTest, TableScanBuilderValidationErrors) {
235289
// Test negative min rows
236290
ICEBERG_UNWRAP_OR_FAIL(auto builder,

0 commit comments

Comments
 (0)