mbutrovich commented on code in PR #2904:
URL: https://github.com/apache/iceberg-rust/pull/2904#discussion_r4133944642
##########
crates/iceberg/src/scan/mod.rs:
##########
@@ -1963,6 +1968,108 @@ pub mod tests {
);
}
+ /// `table` with a column added after its last write, so its current
schema is
+ /// ahead of the schema its current snapshot was written with.
+ fn with_column_added_after_last_write(table: &Table) -> Table {
Review Comment:
What do you think about having this helper take the new schema's fields, and
adding tests for the other schema changes that reach the same branch in
`build`? I tried a few on bec3e42 using the fixture's `dbl` column (id 5). The
fixture's sort order references `z`, so the metadata builder rejects a schema
that drops `z`.
- Drop `dbl` and add a new optional `dbl` with a new field ID, with no write
since. Before this change, `select(["dbl"])` returns the old column's values
(the first batch has 1024 non-null values). With it, the column is all null,
which is what [column
projection](https://github.com/apache/iceberg/blob/48330b8dacab6662242d252b39c8444190979bb2/format/spec.md?plain=1#L401-L409)
in the spec requires, because columns in data files are matched by field ID
and the new field ID isn't in any file. This one is a silent wrong result, so I
think it's the most important case to cover.
- Rename `dbl` to `dbl2`. Before this change, `select(["dbl2"])` fails with
`Column dbl2 not found in table`. With it, the values come back under the new
name.
- Drop `dbl`. This is the behavior change the PR description calls out. A
current-state scan now fails with `Column dbl not found in table`, and a scan
pinned to the current snapshot still reads all 2048 rows. A test would make
that change explicit.
- Filter on the added column. Before this change,
`with_filter(Reference::new("added_after_write").is_null())` fails in `build`
with `Field added_after_write not found in schema`. With it, `is_null()`
returns all 2048 rows and `equal_to(Datum::long(1))` returns none.
--
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]
