blackmwk commented on code in PR #2894:
URL: https://github.com/apache/iceberg-rust/pull/2894#discussion_r3664427659
##########
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:
+1
##########
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:
I'm not a big fan of doing it this way, it's hard to extend and maitain.
Ideally, we should have a static map in compression file, like
TABLE_METADATA_SUFFIX_TO_COMPRESSION, TABLE_METADATA_MAGIC_TO_COMPRESSION,
TABLE_METADATA_SUPPORTED_COMPreSSION, and reuse them in all places.
--
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]
