kevinjqliu commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2548516186
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -175,6 +196,19 @@ impl TryFrom<&HashMap<String, String>> for TableProperties
{
TableProperties::PROPERTY_WRITE_TARGET_FILE_SIZE_BYTES,
TableProperties::PROPERTY_WRITE_TARGET_FILE_SIZE_BYTES_DEFAULT,
)?,
+ avro_compression_codec: parse_property(
+ props,
+ TableProperties::PROPERTY_AVRO_COMPRESSION_CODEC,
+
TableProperties::PROPERTY_AVRO_COMPRESSION_CODEC_DEFAULT.to_string(),
+ )?,
+ avro_compression_level: {
+ let level = parse_property(
+ props,
+ TableProperties::PROPERTY_AVRO_COMPRESSION_LEVEL,
+ 255u8,
+ )?;
+ if level == 255 { None } else { Some(level) }
+ },
Review Comment:
is this not valid here?
```
avro_compression_level: parse_property(
props,
TableProperties::PROPERTY_AVRO_COMPRESSION_LEVEL,
TableProperties::PROPERTY_AVRO_COMPRESSION_LEVEL_DEFAULT,
)?,
```
`avro_compression_level` and `PROPERTY_AVRO_COMPRESSION_LEVEL_DEFAULT` are
both `Option<u8>`
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -49,6 +56,10 @@ pub struct TableProperties {
pub write_format_default: String,
/// The target file size for files.
pub write_target_file_size_bytes: usize,
+ /// Compression codec for Avro files (manifests, manifest lists)
+ pub avro_compression_codec: String,
+ /// Compression level for Avro files (None uses codec-specific defaults:
gzip=9, zstd=1)
Review Comment:
```suggestion
/// Compression level for Avro files (None uses codec-specific defaults)
```
nit: these might change in the future, so i think its better to not specify
the values here
there are a few other instances of this same comment
##########
crates/iceberg/src/spec/avro_util.rs:
##########
@@ -0,0 +1,308 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Utilities for working with Apache Avro in Iceberg.
+
+use apache_avro::Codec;
+use log::warn;
+
+/// Settings for compression codec and level.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct CompressionSettings {
+ /// The compression codec name (e.g., "gzip", "zstd", "deflate",
"uncompressed")
+ pub codec: String,
+ /// The compression level (None uses codec-specific defaults: gzip=9,
zstd=1)
+ pub level: Option<u8>,
+}
+
+impl CompressionSettings {
+ /// Create a new CompressionSettings with the specified codec and level.
+ pub fn new(codec: String, level: Option<u8>) -> Self {
+ Self { codec, level }
+ }
+
+ /// Convert to apache_avro::Codec using the codec_from_str helper function.
+ pub(crate) fn to_codec(&self) -> Codec {
+ codec_from_str(Some(&self.codec), self.level)
+ }
+}
+
+impl Default for CompressionSettings {
+ fn default() -> Self {
+ use crate::spec::TableProperties;
+ Self {
+ codec:
TableProperties::PROPERTY_AVRO_COMPRESSION_CODEC_DEFAULT.to_string(),
+ level: None,
+ }
+ }
+}
+
+/// Convert codec name and level to apache_avro::Codec.
+/// Returns Codec::Null for unknown or unsupported codecs.
+///
+/// # Arguments
+///
+/// * `codec` - The name of the compression codec (e.g., "gzip", "zstd",
"deflate", "uncompressed")
+/// * `level` - The compression level (None uses codec defaults: gzip=9,
zstd=1). For deflate/gzip:
+/// - 0: NoCompression
+/// - 1: BestSpeed
+/// - 9: BestCompression
+/// - 10: UberCompression
+/// - Other values: DefaultLevel (6)
+///
+/// # Supported Codecs
+///
+/// - `gzip` or `deflate`: Uses Deflate compression with specified level
(default: 9)
+/// - `zstd`: Uses Zstandard compression (default: 1, level clamped to valid
zstd range 0-22)
+/// - `uncompressed` or `None`: No compression
+/// - Any other value: Defaults to no compression (Codec::Null)
+///
+/// # Compression Levels
+///
+/// The compression level mapping is based on miniz_oxide's CompressionLevel
enum:
Review Comment:
nit: maybe merge this comment with the one above (L61-L66), the compression
levels are explained twice
##########
crates/iceberg/src/spec/avro_util.rs:
##########
@@ -0,0 +1,310 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Utilities for working with Apache Avro in Iceberg.
+
+use apache_avro::Codec;
+use log::warn;
+
+use crate::spec::TableProperties;
+
+/// Settings for compression codec and level.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct CompressionSettings {
+ /// The compression codec name (e.g., "gzip", "zstd", "snappy",
"uncompressed")
+ pub codec: String,
+ /// The compression level (None uses codec-specific defaults: gzip=9,
zstd=1)
+ pub level: Option<u8>,
+}
+
+impl CompressionSettings {
+ /// Create a new CompressionSettings with the specified codec and level.
+ pub fn new(codec: String, level: Option<u8>) -> Self {
+ Self { codec, level }
+ }
+
+ /// Convert to apache_avro::Codec using the codec_from_str helper function.
+ pub(crate) fn to_codec(&self) -> Codec {
+ codec_from_str(Some(&self.codec), self.level)
+ }
+}
+
+impl Default for CompressionSettings {
+ fn default() -> Self {
+ Self {
+ codec:
TableProperties::PROPERTY_AVRO_COMPRESSION_CODEC_DEFAULT.to_string(),
+ level: None,
Review Comment:
```suggestion
level: TableProperties::PROPERTY_AVRO_COMPRESSION_LEVEL_DEFAULT,
```
--
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]
