You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "emkornfield (via GitHub)" <gi...@apache.org> on 2025/11/13 00:54:01 UTC
[PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
emkornfield opened a new pull request, #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851
## Which issue does this PR close?
- Closes #1731
## What changes are included in this PR?
Previously these properties where not honored on tabel properties.
- Adds table properties for these values.
- Plumbs them through for writers.
## Are these changes tested?
Added unit tests
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2543643006
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -34,6 +34,25 @@ where
})
}
+// Helper function to parse an optional property from a HashMap
+// If the property is not found, returns None
+fn parse_optional_property<T: std::str::FromStr>(
+ properties: &HashMap<String, String>,
+ key: &str,
+) -> Result<Option<T>, anyhow::Error>
Review Comment:
I'll see if I can make this work with empty string.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2530464555
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -461,9 +461,57 @@ impl TableMetadata {
file_io: &FileIO,
metadata_location: impl AsRef<str>,
) -> Result<()> {
+ use std::io::Write as _;
+
+ use flate2::write::GzEncoder;
+
+ let json_data = serde_json::to_vec(self)?;
+
+ // Check if compression is enabled via table properties
+ let codec = self
+ .properties
+ .get(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC)
+ .map(|s| s.as_str())
+ .unwrap_or(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC_DEFAULT);
+
+ let (data_to_write, actual_location) = match codec {
+ "gzip" => {
+ let mut encoder = GzEncoder::new(Vec::new(), flate2::Compression::default());
+ encoder.write_all(&json_data).map_err(|e| {
+ Error::new(
+ ErrorKind::DataInvalid,
+ "Failed to compress metadata with gzip",
+ )
+ .with_source(e)
+ })?;
+ let compressed_data = encoder.finish().map_err(|e| {
+ Error::new(ErrorKind::DataInvalid, "Failed to finish gzip compression")
+ .with_source(e)
+ })?;
+
+ // Modify filename to add .gz before .metadata.json
+ let location = metadata_location.as_ref();
+ let new_location = if location.ends_with(".metadata.json") {
+ location.replace(".metadata.json", ".gz.metadata.json")
Review Comment:
I disagree, metadata.json.gz is only readable in Java as a fallback it generated the `.gz.metadata.json` files. Python [currently requires](https://github.com/apache/iceberg-python/blob/f2131e6eea6fdd1158471e163e21c29dd23a3849/pyiceberg/serializers.py#L35) `.gz.metadata.json`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (including manifest and manifest lists) [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#issuecomment-3832845150
Moving to draft until we finalize https://github.com/apache/iceberg-rust/pull/1876
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2537368596
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -34,6 +34,25 @@ where
})
}
+// Helper function to parse an optional property from a HashMap
+// If the property is not found, returns None
+fn parse_optional_property<T: std::str::FromStr>(
+ properties: &HashMap<String, String>,
+ key: &str,
+) -> Result<Option<T>, anyhow::Error>
Review Comment:
Do we really need this method? I think passing a `None` to default value in `parse_property` would be enough?
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -461,9 +461,59 @@ impl TableMetadata {
file_io: &FileIO,
metadata_location: impl AsRef<str>,
) -> Result<()> {
+ use std::io::Write as _;
+
+ use flate2::write::GzEncoder;
Review Comment:
Why not put the imports at the beginning of file as others do?
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -34,6 +34,25 @@ where
})
}
+// Helper function to parse an optional property from a HashMap
+// If the property is not found, returns None
+fn parse_optional_property<T: std::str::FromStr>(
+ properties: &HashMap<String, String>,
+ key: &str,
+) -> Result<Option<T>, anyhow::Error>
Review Comment:
We should use this crate's error.
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -461,9 +461,59 @@ impl TableMetadata {
file_io: &FileIO,
metadata_location: impl AsRef<str>,
) -> Result<()> {
+ use std::io::Write as _;
+
+ use flate2::write::GzEncoder;
+
+ let json_data = serde_json::to_vec(self)?;
+
+ // Check if compression is enabled via table properties
+ let codec = self
+ .properties
+ .get(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC)
Review Comment:
Why we need a fully qualified name here? It's difficult to read.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (inlcuding manifest and manifest lists) [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2550682509
##########
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:
removing this.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2541212949
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -34,6 +34,25 @@ where
})
}
+// Helper function to parse an optional property from a HashMap
+// If the property is not found, returns None
+fn parse_optional_property<T: std::str::FromStr>(
+ properties: &HashMap<String, String>,
+ key: &str,
+) -> Result<Option<T>, anyhow::Error>
Review Comment:
I think in iceberg all table properties are optional, if they have default values, then we should return a default value, otherwise we should return an error.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2540090997
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -34,6 +34,25 @@ where
})
}
+// Helper function to parse an optional property from a HashMap
+// If the property is not found, returns None
+fn parse_optional_property<T: std::str::FromStr>(
+ properties: &HashMap<String, String>,
+ key: &str,
+) -> Result<Option<T>, anyhow::Error>
Review Comment:
I'm new to rust but would that work? Doesn't parse_property, always return an extension of String instead of an Option<T>? It is useful to know if the value was actually configured (and thus the option return)?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (including manifest and manifest lists) [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2621884120
##########
crates/iceberg/src/spec/manifest/writer.rs:
##########
@@ -43,6 +44,7 @@ pub struct ManifestWriterBuilder {
key_metadata: Option<Vec<u8>>,
schema: SchemaRef,
partition_spec: PartitionSpec,
+ compression: CompressionSettings,
Review Comment:
done.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2530694689
##########
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:
clarify this is for gzip.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2530469094
##########
crates/iceberg/src/transaction/snapshot.rs:
##########
@@ -205,7 +220,9 @@ impl<'a> SnapshotProducer<'a> {
.default_partition_spec()
.as_ref()
.clone(),
- );
+ )
+ .with_compression(compression);
Review Comment:
I don't know that there is strong reason one way or another, happy to do whatever reviewers prefer. As an argument against adding another parameter to the construct, Rust has clippy warnings for too many parameters, so for optional parameters it seems best not to add them?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (inlcuding manifest and manifest lists) [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#issuecomment-3554808001
@liurenjie1024 split of this PR to just be about Avro. Also based on feedback, I cleaned up some additional imports/Errors that weren't using the Crate's error before.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (inlcuding manifest and manifest lists) [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2550513121
##########
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:
I might be misunderstanding the comment. But I should have cleaned up `pub const PROPERTY_AVRO_COMPRESSION_LEVEL_DEFAULT: Option<u8> = None`;, I don't think it is used anymore.
In a prior revision I added a method (`parse_optional_property`) that could return `None` value directly but @liurenjie1024 questioned whether this was useful.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#issuecomment-3525803959
> looks like a clippy issue in CI
Yeah working on a fix.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2530693475
##########
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:
+/// - Level 0: No compression
+/// - Level 1: Best speed (fastest)
+/// - Level 9: Best compression (slower, better compression)
+/// - Level 10: Uber compression (slowest, best compression)
+/// - Other: Default level (balanced speed/compression)
+pub(crate) fn codec_from_str(codec: Option<&str>, level: Option<u8>) -> Codec {
Review Comment:
this can maybe be made private.
##########
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:
+/// - Level 0: No compression
+/// - Level 1: Best speed (fastest)
+/// - Level 9: Best compression (slower, better compression)
+/// - Level 10: Uber compression (slowest, best compression)
+/// - Other: Default level (balanced speed/compression)
+pub(crate) fn codec_from_str(codec: Option<&str>, level: Option<u8>) -> Codec {
Review Comment:
this can maybe be made private.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (inlcuding manifest and manifest lists) [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2621839387
##########
Cargo.toml:
##########
@@ -40,7 +40,7 @@ rust-version = "1.87"
[workspace.dependencies]
anyhow = "1.0.72"
-apache-avro = { version = "0.20", features = ["zstandard"] }
+apache-avro = { version = "0.20", features = ["zstandard", "snappy"] }
Review Comment:
This enable the ability for avro to read/write snappy compressed files.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2532115395
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -461,9 +461,57 @@ impl TableMetadata {
file_io: &FileIO,
metadata_location: impl AsRef<str>,
) -> Result<()> {
+ use std::io::Write as _;
+
+ use flate2::write::GzEncoder;
+
+ let json_data = serde_json::to_vec(self)?;
+
+ // Check if compression is enabled via table properties
+ let codec = self
+ .properties
+ .get(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC)
+ .map(|s| s.as_str())
+ .unwrap_or(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC_DEFAULT);
+
+ let (data_to_write, actual_location) = match codec {
+ "gzip" => {
+ let mut encoder = GzEncoder::new(Vec::new(), flate2::Compression::default());
+ encoder.write_all(&json_data).map_err(|e| {
+ Error::new(
+ ErrorKind::DataInvalid,
+ "Failed to compress metadata with gzip",
+ )
+ .with_source(e)
+ })?;
+ let compressed_data = encoder.finish().map_err(|e| {
+ Error::new(ErrorKind::DataInvalid, "Failed to finish gzip compression")
+ .with_source(e)
+ })?;
+
+ // Modify filename to add .gz before .metadata.json
+ let location = metadata_location.as_ref();
+ let new_location = if location.ends_with(".metadata.json") {
+ location.replace(".metadata.json", ".gz.metadata.json")
Review Comment:
oops yea, youre right
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (including manifest and manifest lists) [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2662723344
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -175,12 +195,38 @@ 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 codec name and level from properties
+ let codec_name = parse_property(
+ props,
+ TableProperties::PROPERTY_AVRO_COMPRESSION_CODEC,
+ TableProperties::PROPERTY_AVRO_COMPRESSION_CODEC_DEFAULT.to_string(),
+ )?;
+
+ // Parse optional compression level (sentinel value 255 means not specified)
+ let level_raw = parse_property(
+ props,
+ TableProperties::PROPERTY_AVRO_COMPRESSION_LEVEL,
+ 255u8,
Review Comment:
maybe make a constant?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (including manifest and manifest lists) [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#issuecomment-4008685334
Still waiting on Moving to draft until we finalize https://github.com/apache/iceberg-rust/pull/1876, I'll resolve conflicts.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2543685332
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -461,9 +461,59 @@ impl TableMetadata {
file_io: &FileIO,
metadata_location: impl AsRef<str>,
) -> Result<()> {
+ use std::io::Write as _;
+
+ use flate2::write::GzEncoder;
Review Comment:
Still new to rust, I thought there was some convention to try to limit scope for rare imports. I've moved this to the top.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (including manifest and manifest lists) [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#issuecomment-4617691147
Yes, sorry I've been delayed in getting this ready for a re-review.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (including manifest and manifest lists) [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2625052500
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -2044,4 +2070,97 @@ mod test {
assert_eq!(v2_manifest.partitions, None);
assert_eq!(v2_manifest.key_metadata, None);
}
+
+ #[tokio::test]
+ async fn test_manifest_list_writer_with_compression() {
+ use std::fs;
Review Comment:
move these to top level.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#issuecomment-3525019740
looks like a clippy issue in CI
```
error: this function has too many arguments (8/7)
--> crates/iceberg/src/spec/manifest_list.rs:243:5
|
243 | / fn new(
244 | | format_version: FormatVersion,
245 | | output_file: OutputFile,
246 | | metadata: HashMap<String, String>,
... |
251 | | compression_level: u8,
252 | | ) -> Self {
| |_____________^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
= note: `-D clippy::too-many-arguments` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::too_many_arguments)]`
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#issuecomment-3529075492
@liurenjie1024 @kevinjqliu would you have time to review?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2535641523
##########
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:
+/// - Level 0: No compression
+/// - Level 1: Best speed (fastest)
+/// - Level 9: Best compression (slower, better compression)
+/// - Level 10: Uber compression (slowest, best compression)
+/// - Other: Default level (balanced speed/compression)
+pub(crate) fn codec_from_str(codec: Option<&str>, level: Option<u8>) -> Codec {
+ use apache_avro::{DeflateSettings, ZstandardSettings};
+
+ match codec {
Review Comment:
yep. done.
##########
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")
Review Comment:
removed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2530685441
##########
crates/iceberg/src/transaction/snapshot.rs:
##########
@@ -205,7 +220,9 @@ impl<'a> SnapshotProducer<'a> {
.default_partition_spec()
.as_ref()
.clone(),
- );
+ )
+ .with_compression(compression);
Review Comment:
This also fixes the bug mentioned below.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#issuecomment-3544026699
@kevinjqliu thanks for the reviews, I also added more e2e tests for manifest and manifest list writers to confirm compression.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2530109818
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -198,23 +202,52 @@ impl ManifestListWriter {
sequence_number,
snapshot_id,
first_row_id,
+ CompressionSettings::default(),
)
}
+ /// Set compression settings for the manifest list file.
+ pub fn with_compression(mut self, compression: CompressionSettings) -> Self {
+ self.compression = compression.clone();
+
+ // Recreate the avro_writer with the new codec
+ let avro_schema = match self.format_version {
+ FormatVersion::V1 => &MANIFEST_LIST_AVRO_SCHEMA_V1,
+ FormatVersion::V2 => &MANIFEST_LIST_AVRO_SCHEMA_V2,
+ FormatVersion::V3 => &MANIFEST_LIST_AVRO_SCHEMA_V3,
+ };
+
+ // Use CompressionSettings to get codec
+ let codec = compression.to_codec();
+
+ let new_writer = Writer::with_codec(avro_schema, Vec::new(), codec);
+
+ // Copy over existing metadata from the old writer
+ // Unfortunately, we can't extract metadata from the old writer,
+ // so we'll need to handle this differently
+ self.avro_writer = new_writer;
Review Comment:
im a bit confused by this comment, do we need to copy over the metadata?
similar to
https://github.com/apache/iceberg-rust/blob/51e781ea5cf85e37791965a8f81bb26496bbaf1e/crates/iceberg/src/spec/manifest_list.rs#L250-L255
##########
crates/iceberg/src/spec/avro_util.rs:
##########
@@ -0,0 +1,280 @@
+// 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", "none")
+ pub codec: String,
+ /// The compression level
+ pub level: u8,
+}
+
+impl CompressionSettings {
+ /// Create a new CompressionSettings with the specified codec and level.
+ pub fn new(codec: String, level: 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: TableProperties::PROPERTY_AVRO_COMPRESSION_LEVEL_DEFAULT,
+ }
+ }
+}
+
+/// 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", "none")
+/// * `level` - The compression level. 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
+/// - `zstd`: Uses Zstandard compression (level clamped to valid zstd range)
+/// - `none` 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:
+/// - Level 0: No compression
+/// - Level 1: Best speed (fastest)
+/// - Level 9: Best compression (slower, better compression)
+/// - Level 10: Uber compression (slowest, best compression)
+/// - Other: Default level (balanced speed/compression)
+pub(crate) fn codec_from_str(codec: Option<&str>, level: u8) -> Codec {
+ use apache_avro::{DeflateSettings, ZstandardSettings};
+
+ match codec {
+ Some("gzip") | Some("deflate") => {
+ // Map compression level to miniz_oxide::deflate::CompressionLevel
+ // Reference: https://docs.rs/miniz_oxide/latest/miniz_oxide/deflate/enum.CompressionLevel.html
+ use miniz_oxide::deflate::CompressionLevel;
+
+ let compression_level = match level {
+ 0 => CompressionLevel::NoCompression,
+ 1 => CompressionLevel::BestSpeed,
+ 9 => CompressionLevel::BestCompression,
+ 10 => CompressionLevel::UberCompression,
+ _ => CompressionLevel::DefaultLevel,
+ };
+
+ Codec::Deflate(DeflateSettings::new(compression_level))
+ }
+ Some("zstd") => {
+ // Zstandard supports levels 0-22, clamp to valid range
+ let zstd_level = level.min(22);
Review Comment:
btw java sets default zstd compression level to 1 but i think its fine to diverge here
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -137,6 +143,21 @@ impl TableProperties {
pub const PROPERTY_WRITE_TARGET_FILE_SIZE_BYTES: &str = "write.target-file-size-bytes";
/// Default target file size
pub const PROPERTY_WRITE_TARGET_FILE_SIZE_BYTES_DEFAULT: usize = 512 * 1024 * 1024; // 512 MB
+
+ /// Compression codec for metadata files (JSON)
+ pub const PROPERTY_METADATA_COMPRESSION_CODEC: &str = "write.metadata.compression-codec";
+ /// Default metadata compression codec - none
+ pub const PROPERTY_METADATA_COMPRESSION_CODEC_DEFAULT: &str = "none";
+
+ /// Compression codec for Avro files (manifests, manifest lists)
+ pub const PROPERTY_AVRO_COMPRESSION_CODEC: &str = "write.avro.compression-codec";
+ /// Default Avro compression codec - gzip
+ pub const PROPERTY_AVRO_COMPRESSION_CODEC_DEFAULT: &str = "gzip";
+
+ /// Compression level for Avro files
+ pub const PROPERTY_AVRO_COMPRESSION_LEVEL: &str = "write.avro.compression-level";
+ /// Default Avro compression level (9 = BestCompression)
+ pub const PROPERTY_AVRO_COMPRESSION_LEVEL_DEFAULT: u8 = 9;
Review Comment:
In the [Iceberg docs](https://iceberg.apache.org/docs/nightly/configuration/#write-properties),
```
write.avro.compression-level null Avro compression level
```
Each compression codec has its own default value [here](https://github.com/apache/iceberg/blob/571b696106f4748d3edee9ee641e214377ddcb16/core/src/main/java/org/apache/iceberg/avro/Avro.java#L259-L267)
And gzip defaults to level 9
Should we follow the same pattern?
##########
crates/iceberg/src/spec/avro_util.rs:
##########
@@ -0,0 +1,280 @@
+// 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", "none")
+ pub codec: String,
+ /// The compression level
+ pub level: u8,
+}
+
+impl CompressionSettings {
+ /// Create a new CompressionSettings with the specified codec and level.
+ pub fn new(codec: String, level: 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: TableProperties::PROPERTY_AVRO_COMPRESSION_LEVEL_DEFAULT,
+ }
+ }
+}
+
+/// 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", "none")
+/// * `level` - The compression level. 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
+/// - `zstd`: Uses Zstandard compression (level clamped to valid zstd range)
+/// - `none` or `None`: No compression
Review Comment:
i think this should be `uncompressed` instead of `none`, to match the same string java uses
https://github.com/apache/iceberg/blob/571b696106f4748d3edee9ee641e214377ddcb16/core/src/main/java/org/apache/iceberg/avro/Avro.java#L252-L267
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -461,9 +461,57 @@ impl TableMetadata {
file_io: &FileIO,
metadata_location: impl AsRef<str>,
) -> Result<()> {
+ use std::io::Write as _;
+
+ use flate2::write::GzEncoder;
+
+ let json_data = serde_json::to_vec(self)?;
+
+ // Check if compression is enabled via table properties
+ let codec = self
+ .properties
+ .get(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC)
+ .map(|s| s.as_str())
+ .unwrap_or(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC_DEFAULT);
+
+ let (data_to_write, actual_location) = match codec {
+ "gzip" => {
+ let mut encoder = GzEncoder::new(Vec::new(), flate2::Compression::default());
+ encoder.write_all(&json_data).map_err(|e| {
+ Error::new(
+ ErrorKind::DataInvalid,
+ "Failed to compress metadata with gzip",
+ )
+ .with_source(e)
+ })?;
+ let compressed_data = encoder.finish().map_err(|e| {
+ Error::new(ErrorKind::DataInvalid, "Failed to finish gzip compression")
+ .with_source(e)
+ })?;
+
+ // Modify filename to add .gz before .metadata.json
+ let location = metadata_location.as_ref();
+ let new_location = if location.ends_with(".metadata.json") {
+ location.replace(".metadata.json", ".gz.metadata.json")
Review Comment:
đŸ˜„ https://iceberg.apache.org/spec/#naming-for-gzip-compressed-metadata-json-files
i wonder if its a good idea to just default to `metadata.json.gz` here and not create more files with the `.gz.metadata.json` suffix
##########
crates/iceberg/src/transaction/snapshot.rs:
##########
@@ -205,7 +220,9 @@ impl<'a> SnapshotProducer<'a> {
.default_partition_spec()
.as_ref()
.clone(),
- );
+ )
+ .with_compression(compression);
Review Comment:
maybe this is a rust stylistic thing, but curious why use this pattern instead of adding an optional `compression` param to `ManifestWriterBuilder`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2530618475
##########
crates/iceberg/src/transaction/snapshot.rs:
##########
@@ -205,7 +220,9 @@ impl<'a> SnapshotProducer<'a> {
.default_partition_spec()
.as_ref()
.clone(),
- );
+ )
+ .with_compression(compression);
Review Comment:
I'll see if I can make it all with constructors to make it consistent.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2530603905
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -198,23 +202,52 @@ impl ManifestListWriter {
sequence_number,
snapshot_id,
first_row_id,
+ CompressionSettings::default(),
)
}
+ /// Set compression settings for the manifest list file.
+ pub fn with_compression(mut self, compression: CompressionSettings) -> Self {
+ self.compression = compression.clone();
+
+ // Recreate the avro_writer with the new codec
+ let avro_schema = match self.format_version {
+ FormatVersion::V1 => &MANIFEST_LIST_AVRO_SCHEMA_V1,
+ FormatVersion::V2 => &MANIFEST_LIST_AVRO_SCHEMA_V2,
+ FormatVersion::V3 => &MANIFEST_LIST_AVRO_SCHEMA_V3,
+ };
+
+ // Use CompressionSettings to get codec
+ let codec = compression.to_codec();
+
+ let new_writer = Writer::with_codec(avro_schema, Vec::new(), codec);
+
+ // Copy over existing metadata from the old writer
+ // Unfortunately, we can't extract metadata from the old writer,
+ // so we'll need to handle this differently
+ self.avro_writer = new_writer;
Review Comment:
Yeah, left over TODO, i'll fix this.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2530482656
##########
crates/iceberg/src/spec/avro_util.rs:
##########
@@ -0,0 +1,280 @@
+// 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", "none")
+ pub codec: String,
+ /// The compression level
+ pub level: u8,
+}
+
+impl CompressionSettings {
+ /// Create a new CompressionSettings with the specified codec and level.
+ pub fn new(codec: String, level: 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: TableProperties::PROPERTY_AVRO_COMPRESSION_LEVEL_DEFAULT,
+ }
+ }
+}
+
+/// 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", "none")
+/// * `level` - The compression level. 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
+/// - `zstd`: Uses Zstandard compression (level clamped to valid zstd range)
+/// - `none` 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:
+/// - Level 0: No compression
+/// - Level 1: Best speed (fastest)
+/// - Level 9: Best compression (slower, better compression)
+/// - Level 10: Uber compression (slowest, best compression)
+/// - Other: Default level (balanced speed/compression)
+pub(crate) fn codec_from_str(codec: Option<&str>, level: u8) -> Codec {
+ use apache_avro::{DeflateSettings, ZstandardSettings};
+
+ match codec {
+ Some("gzip") | Some("deflate") => {
+ // Map compression level to miniz_oxide::deflate::CompressionLevel
+ // Reference: https://docs.rs/miniz_oxide/latest/miniz_oxide/deflate/enum.CompressionLevel.html
+ use miniz_oxide::deflate::CompressionLevel;
+
+ let compression_level = match level {
+ 0 => CompressionLevel::NoCompression,
+ 1 => CompressionLevel::BestSpeed,
+ 9 => CompressionLevel::BestCompression,
+ 10 => CompressionLevel::UberCompression,
+ _ => CompressionLevel::DefaultLevel,
+ };
+
+ Codec::Deflate(DeflateSettings::new(compression_level))
+ }
+ Some("zstd") => {
+ // Zstandard supports levels 0-22, clamp to valid range
+ let zstd_level = level.min(22);
Review Comment:
Yes, it does look like the default for ZSTD is 1, not sure on why the divergence here though, happy to change it we want to match exactly with Java, but I agree it probably isn't critical.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (inlcuding manifest and manifest lists) [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2549282238
##########
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 {
Review Comment:
I feel this struct is complete unnecessary. We should just add a `avro_write_codec: Codec` in `TableProperties`, and everything else should just use that field.
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -137,11 +148,21 @@ impl TableProperties {
pub const PROPERTY_WRITE_TARGET_FILE_SIZE_BYTES: &str = "write.target-file-size-bytes";
/// Default target file size
pub const PROPERTY_WRITE_TARGET_FILE_SIZE_BYTES_DEFAULT: usize = 512 * 1024 * 1024; // 512 MB
+
+ /// Compression codec for Avro files (manifests, manifest lists)
+ pub const PROPERTY_AVRO_COMPRESSION_CODEC: &str = "write.avro.compression-codec";
+ /// Default Avro compression codec - gzip
+ pub const PROPERTY_AVRO_COMPRESSION_CODEC_DEFAULT: &str = "gzip";
+
+ /// Compression level for Avro files
+ pub const PROPERTY_AVRO_COMPRESSION_LEVEL: &str = "write.avro.compression-level";
+ /// Default Avro compression level (None, uses codec-specific defaults: gzip=9, zstd=1)
+ pub const PROPERTY_AVRO_COMPRESSION_LEVEL_DEFAULT: Option<u8> = None;
Review Comment:
The spec says there is no default value, I don't think we need this one.
##########
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,
+ }
+ }
+}
+
+/// 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", "snappy", "uncompressed")
+/// * `level` - The compression level (None uses codec defaults: gzip=9, zstd=1). For gzip:
+/// - 0: NoCompression
+/// - 1: BestSpeed
+/// - 9: BestCompression
+/// - 10: UberCompression
+/// - Other values: DefaultLevel (6)
+///
+/// # Supported Codecs
+///
+/// - `gzip`: Uses Deflate compression with specified level (default: 9)
+/// - `zstd`: Uses Zstandard compression (default: 1, level clamped to valid zstd range 0-22)
+/// - `snappy`: Uses Snappy compression (level parameter ignored)
+/// - `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:
+/// - Level 0: No compression
+/// - Level 1: Best speed (fastest)
+/// - Level 9: Best compression (slower, better compression)
+/// - Level 10: Uber compression (slowest, best compression)
+/// - Other: Default level (balanced speed/compression)
+pub(crate) fn codec_from_str(codec: Option<&str>, level: Option<u8>) -> Codec {
+ use apache_avro::{DeflateSettings, ZstandardSettings};
+
+ // Use case-insensitive comparison to match Java implementation
+ match codec.map(|s| s.to_lowercase()).as_deref() {
+ Some("gzip") => {
+ // Map compression level to miniz_oxide::deflate::CompressionLevel
+ // Reference: https://docs.rs/miniz_oxide/latest/miniz_oxide/deflate/enum.CompressionLevel.html
+ // Default level for gzip/deflate is 9 (BestCompression) to match Java
+ use miniz_oxide::deflate::CompressionLevel;
+
+ let compression_level = match level.unwrap_or(9) {
Review Comment:
Ditto.
##########
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,
Review Comment:
```suggestion
pub avro_compression_codec: Codec,
```
##########
crates/iceberg/src/spec/manifest/writer.rs:
##########
@@ -43,6 +44,7 @@ pub struct ManifestWriterBuilder {
key_metadata: Option<Vec<u8>>,
schema: SchemaRef,
partition_spec: PartitionSpec,
+ compression: CompressionSettings,
Review Comment:
This should be just `Codec`.
##########
Cargo.toml:
##########
@@ -40,7 +40,7 @@ rust-version = "1.87"
[workspace.dependencies]
anyhow = "1.0.72"
-apache-avro = { version = "0.20", features = ["zstandard"] }
+apache-avro = { version = "0.20", features = ["zstandard", "snappy"] }
Review Comment:
Why we need this?
##########
crates/iceberg/Cargo.toml:
##########
@@ -67,6 +67,8 @@ flate2 = { workspace = true }
fnv = { workspace = true }
futures = { workspace = true }
itertools = { workspace = true }
+log = { workspace = true }
+miniz_oxide = "0.8"
Review Comment:
We manage dependencies version in root workspace.
##########
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,
+ }
+ }
+}
+
+/// 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", "snappy", "uncompressed")
+/// * `level` - The compression level (None uses codec defaults: gzip=9, zstd=1). For gzip:
+/// - 0: NoCompression
+/// - 1: BestSpeed
+/// - 9: BestCompression
+/// - 10: UberCompression
+/// - Other values: DefaultLevel (6)
+///
+/// # Supported Codecs
+///
+/// - `gzip`: Uses Deflate compression with specified level (default: 9)
+/// - `zstd`: Uses Zstandard compression (default: 1, level clamped to valid zstd range 0-22)
+/// - `snappy`: Uses Snappy compression (level parameter ignored)
+/// - `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:
+/// - Level 0: No compression
+/// - Level 1: Best speed (fastest)
+/// - Level 9: Best compression (slower, better compression)
+/// - Level 10: Uber compression (slowest, best compression)
+/// - Other: Default level (balanced speed/compression)
+pub(crate) fn codec_from_str(codec: Option<&str>, level: Option<u8>) -> Codec {
+ use apache_avro::{DeflateSettings, ZstandardSettings};
+
+ // Use case-insensitive comparison to match Java implementation
+ match codec.map(|s| s.to_lowercase()).as_deref() {
+ Some("gzip") => {
Review Comment:
Use constants rather than raw string.
##########
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,
+ }
+ }
+}
+
+/// 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", "snappy", "uncompressed")
+/// * `level` - The compression level (None uses codec defaults: gzip=9, zstd=1). For gzip:
+/// - 0: NoCompression
+/// - 1: BestSpeed
+/// - 9: BestCompression
+/// - 10: UberCompression
+/// - Other values: DefaultLevel (6)
+///
+/// # Supported Codecs
+///
+/// - `gzip`: Uses Deflate compression with specified level (default: 9)
+/// - `zstd`: Uses Zstandard compression (default: 1, level clamped to valid zstd range 0-22)
+/// - `snappy`: Uses Snappy compression (level parameter ignored)
+/// - `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:
+/// - Level 0: No compression
+/// - Level 1: Best speed (fastest)
+/// - Level 9: Best compression (slower, better compression)
+/// - Level 10: Uber compression (slowest, best compression)
+/// - Other: Default level (balanced speed/compression)
+pub(crate) fn codec_from_str(codec: Option<&str>, level: Option<u8>) -> Codec {
+ use apache_avro::{DeflateSettings, ZstandardSettings};
+
+ // Use case-insensitive comparison to match Java implementation
+ match codec.map(|s| s.to_lowercase()).as_deref() {
+ Some("gzip") => {
+ // Map compression level to miniz_oxide::deflate::CompressionLevel
+ // Reference: https://docs.rs/miniz_oxide/latest/miniz_oxide/deflate/enum.CompressionLevel.html
+ // Default level for gzip/deflate is 9 (BestCompression) to match Java
+ use miniz_oxide::deflate::CompressionLevel;
Review Comment:
Why put importing here?
##########
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)
+ pub avro_compression_level: Option<u8>,
Review Comment:
We don't need this.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2530688535
##########
crates/iceberg/src/spec/avro_util.rs:
##########
@@ -0,0 +1,280 @@
+// 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", "none")
+ pub codec: String,
+ /// The compression level
+ pub level: u8,
+}
+
+impl CompressionSettings {
+ /// Create a new CompressionSettings with the specified codec and level.
+ pub fn new(codec: String, level: 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: TableProperties::PROPERTY_AVRO_COMPRESSION_LEVEL_DEFAULT,
+ }
+ }
+}
+
+/// 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", "none")
+/// * `level` - The compression level. 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
+/// - `zstd`: Uses Zstandard compression (level clamped to valid zstd range)
+/// - `none` 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:
+/// - Level 0: No compression
+/// - Level 1: Best speed (fastest)
+/// - Level 9: Best compression (slower, better compression)
+/// - Level 10: Uber compression (slowest, best compression)
+/// - Other: Default level (balanced speed/compression)
+pub(crate) fn codec_from_str(codec: Option<&str>, level: u8) -> Codec {
+ use apache_avro::{DeflateSettings, ZstandardSettings};
+
+ match codec {
+ Some("gzip") | Some("deflate") => {
+ // Map compression level to miniz_oxide::deflate::CompressionLevel
+ // Reference: https://docs.rs/miniz_oxide/latest/miniz_oxide/deflate/enum.CompressionLevel.html
+ use miniz_oxide::deflate::CompressionLevel;
+
+ let compression_level = match level {
+ 0 => CompressionLevel::NoCompression,
+ 1 => CompressionLevel::BestSpeed,
+ 9 => CompressionLevel::BestCompression,
+ 10 => CompressionLevel::UberCompression,
+ _ => CompressionLevel::DefaultLevel,
+ };
+
+ Codec::Deflate(DeflateSettings::new(compression_level))
+ }
+ Some("zstd") => {
+ // Zstandard supports levels 0-22, clamp to valid range
+ let zstd_level = level.min(22);
Review Comment:
made this default to 1 for ZSTD
##########
crates/iceberg/src/spec/avro_util.rs:
##########
@@ -0,0 +1,280 @@
+// 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", "none")
+ pub codec: String,
+ /// The compression level
+ pub level: u8,
+}
+
+impl CompressionSettings {
+ /// Create a new CompressionSettings with the specified codec and level.
+ pub fn new(codec: String, level: 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: TableProperties::PROPERTY_AVRO_COMPRESSION_LEVEL_DEFAULT,
+ }
+ }
+}
+
+/// 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", "none")
+/// * `level` - The compression level. 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
+/// - `zstd`: Uses Zstandard compression (level clamped to valid zstd range)
+/// - `none` or `None`: No compression
Review Comment:
fixed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2535642280
##########
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")
Review Comment:
needed an additional feature on Avro, but seems doable.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2535639739
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -137,6 +162,21 @@ impl TableProperties {
pub const PROPERTY_WRITE_TARGET_FILE_SIZE_BYTES: &str = "write.target-file-size-bytes";
/// Default target file size
pub const PROPERTY_WRITE_TARGET_FILE_SIZE_BYTES_DEFAULT: usize = 512 * 1024 * 1024; // 512 MB
+
+ /// Compression codec for metadata files (JSON)
+ pub const PROPERTY_METADATA_COMPRESSION_CODEC: &str = "write.metadata.compression-codec";
+ /// Default metadata compression codec - uncompressed
+ pub const PROPERTY_METADATA_COMPRESSION_CODEC_DEFAULT: &str = "uncompressed";
Review Comment:
done.
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -461,9 +461,57 @@ impl TableMetadata {
file_io: &FileIO,
metadata_location: impl AsRef<str>,
) -> Result<()> {
+ use std::io::Write as _;
+
+ use flate2::write::GzEncoder;
+
+ let json_data = serde_json::to_vec(self)?;
+
+ // Check if compression is enabled via table properties
+ let codec = self
+ .properties
+ .get(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC)
+ .map(|s| s.as_str())
+ .unwrap_or(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC_DEFAULT);
+
+ let (data_to_write, actual_location) = match codec {
+ "gzip" => {
+ let mut encoder = GzEncoder::new(Vec::new(), flate2::Compression::default());
+ encoder.write_all(&json_data).map_err(|e| {
+ Error::new(
+ ErrorKind::DataInvalid,
+ "Failed to compress metadata with gzip",
+ )
+ .with_source(e)
+ })?;
+ let compressed_data = encoder.finish().map_err(|e| {
+ Error::new(ErrorKind::DataInvalid, "Failed to finish gzip compression")
+ .with_source(e)
+ })?;
+
+ // Modify filename to add .gz before .metadata.json
+ let location = metadata_location.as_ref();
+ let new_location = if location.ends_with(".metadata.json") {
+ location.replace(".metadata.json", ".gz.metadata.json")
+ } else {
+ // If it doesn't end with expected pattern, just append .gz
+ format!("{}.gz", location)
+ };
+
+ (compressed_data, new_location)
+ }
+ "uncompressed" | "" => (json_data, metadata_location.as_ref().to_string()),
Review Comment:
done.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (inlcuding manifest and manifest lists) [iceberg-rust]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2543690069
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -34,6 +34,25 @@ where
})
}
+// Helper function to parse an optional property from a HashMap
+// If the property is not found, returns None
+fn parse_optional_property<T: std::str::FromStr>(
+ properties: &HashMap<String, String>,
+ key: &str,
+) -> Result<Option<T>, anyhow::Error>
Review Comment:
> I think in iceberg all table properties are optional, if they have default values, then we should return a default value, otherwise we should return an error.
The problem here is that the default value is dependent on the value for compression, it seems baking this into the parsing layer is coupling two concerns (business logic for parsing) and the actual values. To eliminate this method I've used a sentinel value for the old method. Let me know if that is seems OK.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2543692262
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -16,21 +16,25 @@
// under the License.
use std::collections::HashMap;
+use std::fmt::Display;
+use std::str::FromStr;
+
+use crate::error::{Error, ErrorKind};
// Helper function to parse a property from a HashMap
// If the property is not found, use the default value
-fn parse_property<T: std::str::FromStr>(
Review Comment:
this is cleanup because it sounds like the crate's errors are preferred I can revert this.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2532399241
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -137,6 +162,21 @@ impl TableProperties {
pub const PROPERTY_WRITE_TARGET_FILE_SIZE_BYTES: &str = "write.target-file-size-bytes";
/// Default target file size
pub const PROPERTY_WRITE_TARGET_FILE_SIZE_BYTES_DEFAULT: usize = 512 * 1024 * 1024; // 512 MB
+
+ /// Compression codec for metadata files (JSON)
+ pub const PROPERTY_METADATA_COMPRESSION_CODEC: &str = "write.metadata.compression-codec";
+ /// Default metadata compression codec - uncompressed
+ pub const PROPERTY_METADATA_COMPRESSION_CODEC_DEFAULT: &str = "uncompressed";
Review Comment:
```suggestion
/// Default metadata compression codec - uncompressed
pub const PROPERTY_METADATA_COMPRESSION_CODEC_DEFAULT: &str = "none";
```
this should be "none" according to
https://iceberg.apache.org/docs/nightly/configuration/#write-properties
<img width="769" height="68" alt="Image" src="https://github.com/user-attachments/assets/75c8885b-4653-43ce-978b-6573646f6e69" />
##########
crates/iceberg/src/transaction/snapshot.rs:
##########
@@ -410,6 +444,7 @@ impl<'a> SnapshotProducer<'a> {
self.table.metadata().current_snapshot_id(),
next_seq_num,
Some(first_row_id),
+ compression,
Review Comment:
```suggestion
compression.clone(),
```
should this be .clone? or should the rest be without .clone?
##########
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:
+/// - Level 0: No compression
+/// - Level 1: Best speed (fastest)
+/// - Level 9: Best compression (slower, better compression)
+/// - Level 10: Uber compression (slowest, best compression)
+/// - Other: Default level (balanced speed/compression)
+pub(crate) fn codec_from_str(codec: Option<&str>, level: Option<u8>) -> Codec {
+ use apache_avro::{DeflateSettings, ZstandardSettings};
+
+ match codec {
Review Comment:
nit: java uses case insensitive comparison, should we do the same here?
https://github.com/apache/iceberg/blob/700575f6e58c655ba680a0e5cc25d5a88c225d3c/core/src/main/java/org/apache/iceberg/avro/Avro.java#L252
##########
crates/iceberg/src/spec/manifest/writer.rs:
##########
@@ -410,7 +423,11 @@ impl ManifestWriter {
// Manifest schema did not change between V2 and V3
FormatVersion::V2 | FormatVersion::V3 => manifest_schema_v2(&partition_type)?,
};
- let mut avro_writer = AvroWriter::new(&avro_schema, Vec::new());
+
+ // Determine compression codec using CompressionSettings
Review Comment:
```suggestion
```
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -461,9 +461,57 @@ impl TableMetadata {
file_io: &FileIO,
metadata_location: impl AsRef<str>,
) -> Result<()> {
+ use std::io::Write as _;
+
+ use flate2::write::GzEncoder;
+
+ let json_data = serde_json::to_vec(self)?;
+
+ // Check if compression is enabled via table properties
+ let codec = self
+ .properties
+ .get(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC)
+ .map(|s| s.as_str())
+ .unwrap_or(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC_DEFAULT);
+
+ let (data_to_write, actual_location) = match codec {
+ "gzip" => {
+ let mut encoder = GzEncoder::new(Vec::new(), flate2::Compression::default());
+ encoder.write_all(&json_data).map_err(|e| {
+ Error::new(
+ ErrorKind::DataInvalid,
+ "Failed to compress metadata with gzip",
+ )
+ .with_source(e)
+ })?;
+ let compressed_data = encoder.finish().map_err(|e| {
+ Error::new(ErrorKind::DataInvalid, "Failed to finish gzip compression")
+ .with_source(e)
+ })?;
+
+ // Modify filename to add .gz before .metadata.json
+ let location = metadata_location.as_ref();
+ let new_location = if location.ends_with(".metadata.json") {
+ location.replace(".metadata.json", ".gz.metadata.json")
+ } else {
+ // If it doesn't end with expected pattern, just append .gz
+ format!("{}.gz", location)
+ };
+
+ (compressed_data, new_location)
+ }
+ "uncompressed" | "" => (json_data, metadata_location.as_ref().to_string()),
Review Comment:
```suggestion
"none" | "" => (json_data, metadata_location.as_ref().to_string()),
```
##########
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")
Review Comment:
is it easy to add snappy to completely match all the supported codex in java?
##########
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")
Review Comment:
"deflate" is not a valid codec option in java, but gzip uses CodecFactory.deflateCodec
https://github.com/apache/iceberg/blob/700575f6e58c655ba680a0e5cc25d5a88c225d3c/core/src/main/java/org/apache/iceberg/avro/Avro.java#L264-L267
i think we can just remove deflate
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -461,9 +461,57 @@ impl TableMetadata {
file_io: &FileIO,
metadata_location: impl AsRef<str>,
) -> Result<()> {
+ use std::io::Write as _;
+
+ use flate2::write::GzEncoder;
+
+ let json_data = serde_json::to_vec(self)?;
+
+ // Check if compression is enabled via table properties
+ let codec = self
+ .properties
+ .get(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC)
+ .map(|s| s.as_str())
+ .unwrap_or(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC_DEFAULT);
+
+ let (data_to_write, actual_location) = match codec {
+ "gzip" => {
Review Comment:
nit: java uses case insensitive comparison, should we do the same here?
https://github.com/apache/iceberg/blob/700575f6e58c655ba680a0e5cc25d5a88c225d3c/core/src/main/java/org/apache/iceberg/TableMetadataParser.java#L63
##########
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)
Review Comment:
java throws on unknown codex,
https://github.com/apache/iceberg/blob/700575f6e58c655ba680a0e5cc25d5a88c225d3c/core/src/main/java/org/apache/iceberg/avro/Avro.java#L269-L270
but i think its fine if we want to return Codec::Null
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2535588221
##########
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)
Review Comment:
I don't think this is standardized in the spec, so I agree I think it is better to fail open here.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2535637811
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -461,9 +461,57 @@ impl TableMetadata {
file_io: &FileIO,
metadata_location: impl AsRef<str>,
) -> Result<()> {
+ use std::io::Write as _;
+
+ use flate2::write::GzEncoder;
+
+ let json_data = serde_json::to_vec(self)?;
+
+ // Check if compression is enabled via table properties
+ let codec = self
+ .properties
+ .get(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC)
+ .map(|s| s.as_str())
+ .unwrap_or(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC_DEFAULT);
+
+ let (data_to_write, actual_location) = match codec {
+ "gzip" => {
Review Comment:
yeah, meant to add this, I didn't think I rerequested you the review but thanks for being proactive. This is now added
##########
crates/iceberg/src/transaction/snapshot.rs:
##########
@@ -410,6 +444,7 @@ impl<'a> SnapshotProducer<'a> {
self.table.metadata().current_snapshot_id(),
next_seq_num,
Some(first_row_id),
+ compression,
Review Comment:
clone is not needed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2543685712
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -461,9 +461,59 @@ impl TableMetadata {
file_io: &FileIO,
metadata_location: impl AsRef<str>,
) -> Result<()> {
+ use std::io::Write as _;
+
+ use flate2::write::GzEncoder;
+
+ let json_data = serde_json::to_vec(self)?;
+
+ // Check if compression is enabled via table properties
+ let codec = self
+ .properties
+ .get(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC)
Review Comment:
Changed to just use TableProperties
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#issuecomment-3544540013
> adding the "breaking" label since we're modifying a few public functions
I'm not sure that I can access labels as a contributor, I put a breaking change note in the PR description.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2530687902
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -198,23 +202,52 @@ impl ManifestListWriter {
sequence_number,
snapshot_id,
first_row_id,
+ CompressionSettings::default(),
)
}
+ /// Set compression settings for the manifest list file.
+ pub fn with_compression(mut self, compression: CompressionSettings) -> Self {
+ self.compression = compression.clone();
+
+ // Recreate the avro_writer with the new codec
+ let avro_schema = match self.format_version {
+ FormatVersion::V1 => &MANIFEST_LIST_AVRO_SCHEMA_V1,
+ FormatVersion::V2 => &MANIFEST_LIST_AVRO_SCHEMA_V2,
+ FormatVersion::V3 => &MANIFEST_LIST_AVRO_SCHEMA_V3,
+ };
+
+ // Use CompressionSettings to get codec
+ let codec = compression.to_codec();
+
+ let new_writer = Writer::with_codec(avro_schema, Vec::new(), codec);
+
+ // Copy over existing metadata from the old writer
+ // Unfortunately, we can't extract metadata from the old writer,
+ // so we'll need to handle this differently
+ self.avro_writer = new_writer;
Review Comment:
this was a bug, waiting to happen.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2530699261
##########
crates/iceberg/src/spec/manifest/writer.rs:
##########
@@ -410,7 +423,11 @@ impl ManifestWriter {
// Manifest schema did not change between V2 and V3
FormatVersion::V2 | FormatVersion::V3 => manifest_schema_v2(&partition_type)?,
};
- let mut avro_writer = AvroWriter::new(&avro_schema, Vec::new());
+
+ // Determine compression codec using CompressionSettings
Review Comment:
unneeded comment.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for JSON metadata and Avro [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2530518865
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -137,6 +143,21 @@ impl TableProperties {
pub const PROPERTY_WRITE_TARGET_FILE_SIZE_BYTES: &str = "write.target-file-size-bytes";
/// Default target file size
pub const PROPERTY_WRITE_TARGET_FILE_SIZE_BYTES_DEFAULT: usize = 512 * 1024 * 1024; // 512 MB
+
+ /// Compression codec for metadata files (JSON)
+ pub const PROPERTY_METADATA_COMPRESSION_CODEC: &str = "write.metadata.compression-codec";
+ /// Default metadata compression codec - none
+ pub const PROPERTY_METADATA_COMPRESSION_CODEC_DEFAULT: &str = "none";
+
+ /// Compression codec for Avro files (manifests, manifest lists)
+ pub const PROPERTY_AVRO_COMPRESSION_CODEC: &str = "write.avro.compression-codec";
+ /// Default Avro compression codec - gzip
+ pub const PROPERTY_AVRO_COMPRESSION_CODEC_DEFAULT: &str = "gzip";
+
+ /// Compression level for Avro files
+ pub const PROPERTY_AVRO_COMPRESSION_LEVEL: &str = "write.avro.compression-level";
+ /// Default Avro compression level (9 = BestCompression)
+ pub const PROPERTY_AVRO_COMPRESSION_LEVEL_DEFAULT: u8 = 9;
Review Comment:
Yeah I'll update it do so.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (including manifest and manifest lists) [iceberg-rust]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2621871847
##########
crates/iceberg/Cargo.toml:
##########
@@ -67,6 +67,8 @@ flate2 = { workspace = true }
fnv = { workspace = true }
futures = { workspace = true }
itertools = { workspace = true }
+log = { workspace = true }
+miniz_oxide = "0.8"
Review Comment:
done.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (including manifest and manifest lists) [iceberg-rust]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#issuecomment-4008677234
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat!: Support compression codecs for Avro Files (including manifest and manifest lists) [iceberg-rust]
Posted by "CTTY (via GitHub)" <gi...@apache.org>.
CTTY commented on PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#issuecomment-4617476232
Is this work still active?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org