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: [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]
