wirybeaver commented on code in PR #2894:
URL: https://github.com/apache/iceberg-rust/pull/2894#discussion_r3666044259
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -107,22 +107,24 @@ pub(crate) fn parse_metadata_file_compression(
Error::new(
ErrorKind::DataInvalid,
format!(
- "Invalid metadata compression codec: {value}. Only '{}' and
'{}' are supported.",
+ "Invalid metadata compression codec: {value}. Only '{}', '{}',
and '{}' are supported.",
CompressionCodec::None.name(),
- CompressionCodec::gzip_default().name()
+ CompressionCodec::gzip_default().name(),
+ CompressionCodec::zstd_default().name()
Review Comment:
Done in fdc645c. I centralized the supported metadata codecs, suffix
mappings, and magic mappings in `compression.rs`, then reused them for property
validation, location parsing, and content-based detection. The write path now
relies on the validated codec directly as well.
##########
crates/iceberg/src/catalog/metadata_location.rs:
##########
@@ -90,20 +90,34 @@ impl MetadataLocation {
}
/// Parses a file name of the format `<version>-<uuid>.metadata.json`
- /// or with compression: `<version>-<uuid>.gz.metadata.json`.
+ /// or with compression before or after `.metadata.json`.
/// Parse errors for compression codec result in CompressionCodec::None.
fn parse_file_name(file_name: &str) -> Result<(i32, Uuid,
CompressionCodec)> {
- let stripped =
file_name.strip_suffix(".metadata.json").ok_or(Error::new(
- ErrorKind::Unexpected,
- format!("Invalid metadata file ending: {file_name}"),
- ))?;
-
- // Check for compression suffix (e.g., .gz)
let gzip_suffix = CompressionCodec::gzip_default().suffix()?;
- let (stripped, compression_codec) = if let Some(s) =
stripped.strip_suffix(gzip_suffix) {
- (s, CompressionCodec::gzip_default())
+ let zstd_suffix = CompressionCodec::zstd_default().suffix()?;
+ let metadata_suffix = ".metadata.json";
+
+ let (stripped, compression_codec) = if let Some(stripped) =
+ file_name.strip_suffix(&format!("{metadata_suffix}{zstd_suffix}"))
+ {
+ (stripped, CompressionCodec::zstd_default())
+ } else if let Some(stripped) =
+ file_name.strip_suffix(&format!("{metadata_suffix}{gzip_suffix}"))
+ {
+ (stripped, CompressionCodec::gzip_default())
+ } else if let Some(stripped) = file_name.strip_suffix(metadata_suffix)
{
+ if let Some(stripped) = stripped.strip_suffix(zstd_suffix) {
+ (stripped, CompressionCodec::zstd_default())
+ } else if let Some(stripped) = stripped.strip_suffix(gzip_suffix) {
+ (stripped, CompressionCodec::gzip_default())
+ } else {
+ (stripped, CompressionCodec::None)
+ }
} else {
- (stripped, CompressionCodec::None)
+ return Err(Error::new(
+ ErrorKind::Unexpected,
+ format!("Invalid metadata file ending: {file_name}"),
+ ));
Review Comment:
Done in fdc645c. I added the suggested `strip_metadata_suffix` helper and
iterator-based parsing, using the shared suffix-to-codec mapping so the
canonical and trailing forms stay easy to extend.
--
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]
