xanderbailey commented on code in PR #2894:
URL: https://github.com/apache/iceberg-rust/pull/2894#discussion_r3661048518
##########
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:
```suggestion
let codecs = [
CompressionCodec::zstd_default(),
CompressionCodec::gzip_default(),
];
let (stripped, compression_codec) = codecs
.into_iter()
.find_map(|codec| {
strip_metadata_suffix(file_name,
codec.suffix().ok()?).map(|s| (s, codec))
})
.or_else(|| {
file_name
.strip_suffix(METADATA_SUFFIX)
.map(|s| (s, CompressionCodec::None))
})
.ok_or_else(|| {
Error::new(
ErrorKind::Unexpected,
format!("Invalid metadata file ending: {file_name}"),
)
})?;
```
```rust
fn strip_metadata_suffix<'a>(file_name: &'a str, codec_suffix: &str) ->
Option<&'a str> {
file_name
.strip_suffix(METADATA_SUFFIX)
.and_then(|s| s.strip_suffix(codec_suffix))
.or_else(|| {
file_name
.strip_suffix(codec_suffix)
.and_then(|s| s.strip_suffix(METADATA_SUFFIX))
})
}
```
I wonder if this kind of thing reads a little better, this is very nested
and hard to follow as it currently stands
--
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]
