wirybeaver commented on code in PR #2894:
URL: https://github.com/apache/iceberg-rust/pull/2894#discussion_r4017045398
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -43,31 +69,14 @@ fn parse_metadata_compression(value: &str) ->
Result<CompressionCodec> {
let lowercase_value = value.to_lowercase();
// Use serde to parse the codec (which has rename_all = "lowercase")
- let codec: CompressionCodec =
serde_json::from_value(serde_json::Value::String(
- lowercase_value,
- ))
- .map_err(|_| {
- Error::new(
- ErrorKind::DataInvalid,
- format!(
- "Invalid metadata compression codec: {value}. Only '{}' and
'{}' are supported.",
- CompressionCodec::None.name(),
- CompressionCodec::gzip_default().name()
- ),
- )
- })?;
-
- // Validate that only None and Gzip are used for metadata
- match codec {
- CompressionCodec::None | CompressionCodec::Gzip(_) => Ok(codec),
- _ => Err(Error::new(
- ErrorKind::DataInvalid,
- format!(
- "Invalid metadata compression codec: {value}. Only '{}' and
'{}' are supported for metadata files.",
- CompressionCodec::None.name(),
- CompressionCodec::gzip_default().name()
- ),
- )),
+ let codec: CompressionCodec =
+ serde_json::from_value(serde_json::Value::String(lowercase_value))
+ .map_err(|_| invalid_metadata_compression_codec(value))?;
+
+ if TABLE_METADATA_SUPPORTED_COMPRESSION.contains(&codec) {
Review Comment:
Made metadata compression support validation level-aware in 89da77d6.
The check now matches Gzip(_) and Zstd(_) instead of comparing against
default-level enum values. Tests cover non-default Gzip and Zstd levels to
prevent this regression.
[addressed by agent]
##########
crates/iceberg/src/compression.rs:
##########
@@ -62,6 +66,22 @@ pub enum CompressionCodec {
Snappy,
}
+pub(crate) const TABLE_METADATA_SUPPORTED_COMPRESSION: &[CompressionCodec] = &[
+ CompressionCodec::None,
+ CompressionCodec::Gzip(GZIP_DEFAULT_LEVEL),
+ CompressionCodec::Zstd(ZSTD_DEFAULT_LEVEL),
+];
+
+pub(crate) const TABLE_METADATA_SUFFIX_TO_COMPRESSION: &[(&str,
CompressionCodec)] = &[
+ (ZSTD_SUFFIX, CompressionCodec::Zstd(ZSTD_DEFAULT_LEVEL)),
+ (GZIP_SUFFIX, CompressionCodec::Gzip(GZIP_DEFAULT_LEVEL)),
+];
+
+pub(crate) const TABLE_METADATA_MAGIC_TO_COMPRESSION: &[(&[u8],
CompressionCodec)] = &[
+ (GZIP_MAGIC, CompressionCodec::Gzip(GZIP_DEFAULT_LEVEL)),
+ (ZSTD_MAGIC, CompressionCodec::Zstd(ZSTD_DEFAULT_LEVEL)),
+];
Review Comment:
Replaced the static metadata codec maps with associated const helpers on
CompressionCodec in 89da77d6.
The codec list, suffix mapping, and magic mapping now live behind
CompressionCodec methods and are reused by property validation, location
parsing, and content-based reads.
[addressed by agent]
--
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]
