Skip to content

feat(transaction): record the Iceberg-Rust writer version information in snapshot summaries #3363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
RussellSpitzer wants to merge 11 commits into
base: main
Choose a base branch
from
+45 −4
Open

feat(transaction): record the Iceberg-Rust writer version information in snapshot summaries #3363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a312562
feat(transaction): record the Rust writer in snapshot summaries
RussellSpitzer Oct 6, 2026
bc2db59
Remove the writer-identity comments.
RussellSpitzer Oct 6, 2026
0742543
Keep a caller-supplied engine name and version.
RussellSpitzer Oct 6, 2026
a7dc0ce
Keep the writer-identity tests on snapshot properties.
RussellSpitzer Oct 6, 2026
78a92e8
Write only iceberg-version into snapshot summaries.
RussellSpitzer Oct 7, 2026
ba05028
Drop the caller engine-identity snapshot test.
RussellSpitzer Oct 7, 2026
3105575
Stop asserting that engine identity keys are absent.
RussellSpitzer Oct 7, 2026
dc2086e
Update crates/iceberg/src/transaction/append.rs
RussellSpitzer Oct 7, 2026
42578ee
Move the iceberg-version summary key next to the other keys.
RussellSpitzer Oct 7, 2026
5b9c042
Merge the accepted iceberg-version test suggestion.
RussellSpitzer Oct 7, 2026
0679bba
Set iceberg-version from the snapshot summary module.
RussellSpitzer Oct 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions crates/iceberg/src/spec/snapshot_summary.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const TOTAL_RECORDS: &str = "total-records";
const TOTAL_FILE_SIZE: &str = "total-files-size";
const CHANGED_PARTITION_COUNT_PROP: &str = "changed-partition-count";
const CHANGED_PARTITION_PREFIX: &str = "partitions.";
const ICEBERG_VERSION_PROP: &str = "iceberg-version";

/// `SnapshotSummaryCollector` collects and aggregates snapshot update metrics.
/// It gathers metrics about added or removed data files and manifests, and tracks
Expand Down Expand Up @@ -330,6 +331,14 @@ where T: PartialOrd + Default + ToString {
}
}

pub(crate) fn set_iceberg_version(properties: &mut HashMap<String, String>) {

@laskoviymishka laskoviymishka Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This always overwrites a caller-supplied iceberg-version, which is the right precedence — Java and Go both apply environment context after user snapshot properties, so the library value wins. Worth a one-line /// here saying it always overwrites, since silently dropping a caller's value is otherwise a little surprising.

let version = env!("CARGO_PKG_VERSION");
properties.insert(
ICEBERG_VERSION_PROP.to_string(),
format!("Apache Iceberg Rust {version}"),

@laskoviymishka laskoviymishka Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small parity thing: Java and Go seed this as Apache Iceberg <ver> (Java's IcebergBuild.fullVersion() → Apache Iceberg 1.10.0 (commit abc…)), so a parser matching Apache Iceberg (\S+) across the three clients would pull Rust out as the version here. Apache Iceberg {version} (rust) keeps the version in the same slot. It's informational-only today, so not blocking — just easier to align now than after it's in the wild.

);
}
Comment on lines +334 to +340

@anoopj anoopj Oct 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional since this is not in the hot path, but since env!("CARGO_PKG_VERSION") is a compile-time &'static str, the whole value can be a const and skip the per-commit format!. ie something like:

const ICEBERG_VERSION_VALUE: &str = concat!("Apache Iceberg Rust ", env!("CARGO_PKG_VERSION"));

pub(crate) fn set_iceberg_version(properties: &mut HashMap<String, String>) {
    properties.insert(ICEBERG_VERSION_PROP.to_string(), ICEBERG_VERSION_VALUE.to_string());
}


pub(crate) fn update_snapshot_summaries(
summary: Summary,
previous_summary: Option<&Summary>,
Expand Down
31 changes: 31 additions & 0 deletions crates/iceberg/src/transaction/append.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,37 @@ mod tests {
);
}

#[tokio::test]
async fn test_snapshot_summary_records_writer_identity() {
let table = make_v2_minimal_table();
let tx = Transaction::new(&table);

let snapshot_properties = HashMap::from([
("key".to_string(), "val".to_string()),
("iceberg-version".to_string(), "caller".to_string()),
]);

let action = tx
.fast_append()
.set_snapshot_properties(snapshot_properties);
let mut action_commit = Arc::new(action).commit(&table).await.unwrap();
let updates = action_commit.take_updates();

let new_snapshot = if let TableUpdate::AddSnapshot { snapshot } = &updates[0] {
snapshot
} else {
unreachable!()
};
let props = &new_snapshot.summary().additional_properties;
let version = env!("CARGO_PKG_VERSION");

assert_eq!(
props.get("iceberg-version").unwrap(),
&format!("Apache Iceberg Rust {version}")
);
assert_eq!(props.get("key").unwrap(), "val");
}

#[tokio::test]
async fn test_append_snapshot_properties() {
let table = make_v2_minimal_table();
Expand Down
9 changes: 5 additions & 4 deletions crates/iceberg/src/transaction/snapshot.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,11 @@ use crate::spec::{
DataFile, DataFileFormat, FormatVersion, MAIN_BRANCH, ManifestContentType, ManifestEntry,
ManifestFile, ManifestListWriter, ManifestWriter, ManifestWriterBuilder, Operation, Snapshot,
SnapshotReference, SnapshotRetention, SnapshotSummaryCollector, Struct, StructType, Summary,
TableProperties, update_snapshot_summaries,
TableProperties, set_iceberg_version, update_snapshot_summaries,
};
use crate::table::Table;
use crate::transaction::ActionCommit;
use crate::{Error, ErrorKind, TableRequirement, TableUpdate};

@anoopj anoopj Oct 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the removal of this line was unintentional? I thin it will glue the doc comment to the import block. I think cargo fmt won't re-add it. Could you restore the blank line?

/// A trait that defines how different table operations produce new snapshots.

@laskoviymishka laskoviymishka Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the blank line between the use block and this doc comment got dropped — unrelated churn, I'd restore it.

///
/// `SnapshotProduceOperation` is used by [`SnapshotProducer`] to customize snapshot creation
Expand Down Expand Up @@ -411,11 +410,13 @@ impl<'a> SnapshotProducer<'a> {
additional_properties,
};

update_snapshot_summaries(
let mut summary = update_snapshot_summaries(
summary,
previous_snapshot.map(|s| s.summary()),
snapshot_produce_operation.operation() == Operation::Overwrite,
)
)?;
set_iceberg_version(&mut summary.additional_properties);

@RussellSpitzer RussellSpitzer Oct 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbutrovich Moved this into snapshot_summary so we don't have to have public constants

Ok(summary)
}

fn generate_manifest_list_file_path(&self, attempt: i64) -> Result<String> {
Expand Down
Loading