NoahKusaba commented on code in PR #2904:
URL: https://github.com/apache/iceberg-rust/pull/2904#discussion_r4259762779


##########
crates/iceberg/src/scan/mod.rs:
##########
@@ -843,6 +851,248 @@ mod tests {
         );
     }
 
+    /// The fixture table with its data files, after `change` is applied to its
+    /// current schema's fields with no write since. Its current snapshot was 
then
+    /// written with an older schema than its current one.
+    async fn table_with_fields_changed_after_last_write(

Review Comment:
   Done in 1ae3ea7a1: the setup is now 
`TableTestFixture::new_with_column_added_after_last_write()`.



##########
crates/iceberg/src/scan/mod.rs:
##########
@@ -843,6 +851,248 @@ mod tests {
         );
     }
 
+    /// The fixture table with its data files, after `change` is applied to its
+    /// current schema's fields with no write since. Its current snapshot was 
then
+    /// written with an older schema than its current one.
+    async fn table_with_fields_changed_after_last_write(
+        change: impl FnOnce(&mut Vec<NestedFieldRef>),
+    ) -> Table {
+        let mut fixture = TableTestFixture::new();
+        fixture.setup_manifest_files().await;
+        let table = fixture.table;
+
+        let mut fields = table
+            .metadata()
+            .current_schema()
+            .as_struct()
+            .fields()
+            .to_vec();
+        change(&mut fields);
+        let metadata = 
TableMetadataBuilder::new_from_metadata(table.metadata().clone(), None)
+            .add_schema(Schema::builder().with_fields(fields).build().unwrap())
+            .unwrap()
+            .set_current_schema(-1)
+            .unwrap()
+            .build()
+            .unwrap()
+            .metadata;
+
+        Table::builder()
+            .metadata(metadata)
+            .identifier(table.identifier().clone())
+            .file_io(table.file_io().clone())
+            .metadata_location(table.metadata_location().unwrap().to_string())
+            .runtime(test_runtime())
+            .build()
+            .unwrap()
+    }
+
+    async fn read(scan: TableScan) -> Vec<RecordBatch> {

Review Comment:
   Done in 1ae3ea7a1: the two unit tests now only check the scan's snapshot and 
projected schema, for an unpinned scan and a scan pinned to the snapshot.



##########
crates/iceberg/src/scan/mod.rs:
##########
@@ -843,6 +851,248 @@ mod tests {
         );
     }
 
+    /// The fixture table with its data files, after `change` is applied to its
+    /// current schema's fields with no write since. Its current snapshot was 
then
+    /// written with an older schema than its current one.
+    async fn table_with_fields_changed_after_last_write(
+        change: impl FnOnce(&mut Vec<NestedFieldRef>),
+    ) -> Table {
+        let mut fixture = TableTestFixture::new();
+        fixture.setup_manifest_files().await;
+        let table = fixture.table;
+
+        let mut fields = table
+            .metadata()
+            .current_schema()
+            .as_struct()
+            .fields()
+            .to_vec();
+        change(&mut fields);
+        let metadata = 
TableMetadataBuilder::new_from_metadata(table.metadata().clone(), None)
+            .add_schema(Schema::builder().with_fields(fields).build().unwrap())
+            .unwrap()
+            .set_current_schema(-1)
+            .unwrap()
+            .build()
+            .unwrap()
+            .metadata;
+
+        Table::builder()
+            .metadata(metadata)
+            .identifier(table.identifier().clone())
+            .file_io(table.file_io().clone())
+            .metadata_location(table.metadata_location().unwrap().to_string())
+            .runtime(test_runtime())
+            .build()
+            .unwrap()
+    }
+
+    async fn read(scan: TableScan) -> Vec<RecordBatch> {
+        scan.to_arrow().await.unwrap().try_collect().await.unwrap()
+    }
+
+    fn column_names(batches: &[RecordBatch]) -> Vec<String> {

Review Comment:
   Moved them to integration tests in 1ae3ea7a1, then removed them in aaca7b0e6 
(see the thread on that file).



##########
crates/integration_tests/tests/read_schema_changed_after_last_write.rs:
##########


Review Comment:
   Removed in aaca7b0e6, along with the tables it added to `provision.py`. I 
opened apache/datafusion-iceberg#75 to track adding sqllogictests for this 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: [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]

Reply via email to