JeonDaehong commented on code in PR #3253:
URL: https://github.com/apache/iceberg-rust/pull/3253#discussion_r4195092343
##########
crates/iceberg/src/transaction/snapshot.rs:
##########
@@ -396,6 +412,19 @@ impl<'a> SnapshotProducer<'a> {
);
}
+ for data_file in &self.deleted_data_files {
+ let partition_spec = table_metadata
+ .partition_spec_by_id(data_file.partition_spec_id)
+ .ok_or_else(|| {
+ invalid_data!("Unknown partition spec id {}",
data_file.partition_spec_id)
+ })?;
+ summary_collector.remove_file(
+ data_file,
+ table_metadata.current_schema().clone(),
Review Comment:
I tried this on the PR head.
The `partition_type().unwrap()` itself does not fail for a dropped source
column, since #2773 falls back to a source-independent type (`Unknown` for
identity/truncate). However, `partition_to_path` panics further down, in
`Display` for `Datum` (`unreachable!()`), when it tries to build the
human-readable string for an `int`/`long` value with an `Unknown` type.
| Old spec field, source dropped | Rust | Java
`partitionToPath` |
|--------------------------------------------------------|-------|------------------------|
| identity(int) | panic | `p=1`
|
| identity(timestamp) | panic | `p=1`
|
| truncate(int) | panic | `p=10`
|
| identity(string), bucket, truncate(string), day, void | ok | ok
|
So the concern still holds, although the failure occurs at a different point
than initially expected.
A fixture with an identity partition on a dropped `int` column should
reproduce the issue.
It looks like the fix may belong in `Datum` / `to_human_string()` rather
than here. I can open a separate issue for that if that would be helpful.
--
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]
