zeodtr commented on issue #177: URL: https://github.com/apache/iceberg-rust/issues/177#issuecomment-1925160320
In the following code, from: https://github.com/apache/iceberg-rust/blob/9ae9e13fb48ea8af20d76644f27dcb2fc8773396/crates/iceberg/src/spec/table_metadata.rs#L679 ```rust impl From<TableMetadata> for TableMetadataV1 { fn from(v: TableMetadata) -> Self { TableMetadataV1 { format_version: VersionNumber::<1>, table_uuid: Some(v.table_uuid), location: v.location, last_updated_ms: v.last_updated_ms, last_column_id: v.last_column_id, schema: v .schemas .get(&v.current_schema_id) .expect("current_schema_id not found in schemas") .as_ref() .clone() .into(), ``` The code `expect()`s `current_schema_id` should not be `None`. But it is valid to be` None` in `TableMetadataV1`, although such usage is deprecated. (See Iceberg spec https://iceberg.apache.org/spec/#table-metadata-fields) I think that using `expect()` is unacceptable in such a case. Actually, in this case, I think that the code should instead use the `schema` field of `TableMetadataV1` when the `current_schema_id` field is `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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
