This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-2904-582b9e103513f21094f61cd1f09fdbf39dccf13e in repository https://gitbox.apache.org/repos/asf/iceberg-rust.git
commit e948f627352b520bc3a37c76502a7464f9312f86 Author: NoahKusaba <[email protected]> AuthorDate: Thu Oct 15 08:06:17 2026 +0000 fix(scan): don't project a stale schema after a schema update with no write (#2904) * init * cleanup * minimal tests * fix conflicts and cleanup * test(scan): cover each schema change after the last write Test the other schema changes that reach the current-schema branch of TableScanBuilder::build, each through a real read of the fixture's data files and with full assertions on what comes back: - a column dropped and added back under the same name reads as null, where it previously returned the old column's values - a renamed column reads under its new name - a dropped column is no longer selectable from the current state, but a scan pinned to the snapshot still reads it - a filter on an added column binds; is_null matches every row Merge the two current-state tests for an added column into one that reads the column, and assert the full error of the time-travel test. Document on TableScanBuilder::snapshot_id that pinning a snapshot also selects the schema it was written with, and rename PlanContext's snapshot_schema to scan_schema, since it now holds the current schema for a scan of the current state. * refactor(scan): keep PlanContext's snapshot_schema name, and document what it holds * ci: retrigger * test(scan): check snapshot and schema in unit tests, move data checks to integration tests Add TableTestFixture::new_with_column_added_after_last_write and slim the scan unit tests to assert the scan's snapshot and projected schema. The data checks for added, dropped, re-added and renamed columns move to read_schema_changed_after_last_write.rs, against tables provisioned by Spark. * test: remove integration tests for scans after a schema change End-to-end checks are tracked in apache/datafusion-iceberg#75 as sqllogictests. --- crates/iceberg/src/scan/context.rs | 3 +++ crates/iceberg/src/scan/mod.rs | 44 +++++++++++++++++++++++++++++++++-- crates/iceberg/src/test_utils/scan.rs | 30 ++++++++++++++++++++++-- 3 files changed, 73 insertions(+), 4 deletions(-) diff --git a/crates/iceberg/src/scan/context.rs b/crates/iceberg/src/scan/context.rs index d17835afa..211ebf8f0 100644 --- a/crates/iceberg/src/scan/context.rs +++ b/crates/iceberg/src/scan/context.rs @@ -198,6 +198,9 @@ pub(crate) struct PlanContext { pub snapshot: SnapshotRef, pub table_metadata: TableMetadataRef, + /// The schema the scan projects. For a scan of the current state, this is + /// the table's current schema, not the snapshot's; for time travel, it is + /// the schema the snapshot was written with. pub snapshot_schema: SchemaRef, pub case_sensitive: bool, pub predicate: Option<Arc<Predicate>>, diff --git a/crates/iceberg/src/scan/mod.rs b/crates/iceberg/src/scan/mod.rs index 7e81900a5..6d23e9470 100644 --- a/crates/iceberg/src/scan/mod.rs +++ b/crates/iceberg/src/scan/mod.rs @@ -211,7 +211,9 @@ impl<'a> TableScanBuilder<'a> { self } - /// Set the snapshot to scan. When not set, it uses current snapshot. + /// Set the snapshot to scan. The scan projects the schema that snapshot was + /// written with. When not set, the scan reads the current snapshot with the + /// table's current schema. pub fn snapshot_id(mut self, snapshot_id: i64) -> Self { self.snapshot_id = Some(snapshot_id); self @@ -313,7 +315,12 @@ impl<'a> TableScanBuilder<'a> { } }; - let schema = snapshot.schema(self.table.metadata())?; + // A scan of the current state uses the table's current schema; time travel uses the + // schema its snapshot was written with. + let schema = match self.snapshot_id { + Some(_) => snapshot.schema(self.table.metadata())?, + None => self.table.metadata().current_schema().clone(), + }; let field_ids = collect_scan_field_ids(&schema, self.column_names.as_deref(), self.case_sensitive)?; let snapshot_bound_predicate = @@ -899,6 +906,39 @@ mod tests { ); } + /// A scan of the current state reads the current snapshot with the table's + /// current schema, not the older one that snapshot was written with. + #[test] + fn test_scan_of_current_state_uses_current_schema() { + let table = TableTestFixture::new_with_column_added_after_last_write().table; + + let table_scan = table.scan().build().unwrap(); + assert_eq!(table_scan.snapshot(), table.metadata().current_snapshot()); + assert_eq!( + table_scan.plan_context.as_ref().unwrap().snapshot_schema, + *table.metadata().current_schema() + ); + } + + /// A scan pinned to a snapshot uses the schema that snapshot was written + /// with, even when it is the current snapshot. + #[test] + fn test_time_travel_scan_uses_snapshot_schema() { + let table = TableTestFixture::new_with_column_added_after_last_write().table; + let snapshot = table.metadata().current_snapshot().unwrap(); + + let table_scan = table + .scan() + .snapshot_id(snapshot.snapshot_id()) + .build() + .unwrap(); + assert_eq!(table_scan.snapshot(), Some(snapshot)); + assert_eq!( + table_scan.plan_context.as_ref().unwrap().snapshot_schema, + snapshot.schema(table.metadata()).unwrap() + ); + } + fn table_with_property(key: &str, value: &str) -> Table { let fixture = TableTestFixture::new(); let mut metadata = fixture.table.metadata().clone(); diff --git a/crates/iceberg/src/test_utils/scan.rs b/crates/iceberg/src/test_utils/scan.rs index 0c4a7e9dd..63f6418ef 100644 --- a/crates/iceberg/src/test_utils/scan.rs +++ b/crates/iceberg/src/test_utils/scan.rs @@ -43,8 +43,8 @@ use crate::metadata_columns::{ }; use crate::spec::{ DataContentType, DataFileBuilder, DataFileFormat, FormatVersion, Literal, ManifestEntry, - ManifestListWriter, ManifestStatus, ManifestWriterBuilder, PartitionSpec, Struct, StructType, - TableMetadata, TableMetadataBuilder, + ManifestListWriter, ManifestStatus, ManifestWriterBuilder, NestedField, PartitionSpec, + PrimitiveType, Schema, Struct, StructType, TableMetadata, TableMetadataBuilder, Type, }; use crate::table::Table; use crate::test_utils::test_runtime; @@ -267,6 +267,32 @@ impl TableTestFixture { } } + /// Creates a fixture with a column added to its schema after its last + /// write, so its current snapshot was written with an older schema than + /// its current one. + pub(crate) fn new_with_column_added_after_last_write() -> Self { + let fixture = Self::new(); + let metadata = fixture.table.metadata(); + + let mut fields = metadata.current_schema().as_struct().fields().to_vec(); + // Beyond the fixture's field ids 1..=8. + fields + .push(NestedField::optional(100, "added", Type::Primitive(PrimitiveType::Long)).into()); + let new_table_metadata = TableMetadataBuilder::new_from_metadata(metadata.clone(), None) + .add_schema(Schema::builder().with_fields(fields).build().unwrap()) + .unwrap() + .set_current_schema(-1) + .unwrap() + .build() + .unwrap() + .metadata; + + Self { + table: fixture.table.with_metadata(Arc::new(new_table_metadata)), + ..fixture + } + } + fn next_manifest_file(&self) -> OutputFile { self.table .file_io()
