Skip to content

Conversation

@Li0k
Copy link

@Li0k Li0k commented Sep 24, 2025

pick feat(iceberg): introduce remove snapshot action (#21)

@Li0k Li0k force-pushed the li0k/pick_82725fd1_to_dev_0808 branch 2 times, most recently from db988d4 to a063986 Compare September 24, 2025 10:08
@chenzl25 chenzl25 requested a review from Copilot September 25, 2025 05:19
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces the remove snapshot action feature to the Iceberg library, enabling snapshot expiration and cleanup operations. The implementation adds functionality to expire snapshots based on age, retention count, and specific snapshot IDs.

  • Adds RemoveSnapshotAction transaction action with configurable expiration policies
  • Implements snapshot retention logic based on branch references and age thresholds
  • Provides integration tests for the new snapshot expiration functionality

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crates/iceberg/src/transaction/remove_snapshots.rs Core implementation of the remove snapshots action with retention logic
crates/iceberg/src/transaction/mod.rs Exposes the new action through the transaction API
crates/iceberg/src/spec/table_metadata.rs Adds constants for snapshot expiration configuration
crates/iceberg/src/spec/snapshot.rs Adds ancestor iteration utilities for snapshot traversal
crates/integration_tests/tests/shared_tests/remove_snapshots_test.rs Integration test for the new functionality
crates/integration_tests/tests/shared_tests/mod.rs Registers the new test module
crates/integration_tests/Cargo.toml Adds chrono dependency for timestamp handling
crates/integration_tests/tests/shared_tests/append_data_file_test.rs Fixes data file writer reuse issue
crates/iceberg/testdata/table_metadata/TableMetadataV2ValidMultiSnapshot.json Test data with multiple snapshots

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Collaborator

@chenzl25 chenzl25 left a comment

Choose a reason for hiding this comment

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

Rest LGTM

* feat(iceberg): basic remove snapshot

* feat(iceberg): introduce new properties for remove snapshots

* feat(iceberg): support remove schemas

* refactor(iceberg): refactor file org

* address comments

* refactor(iceberg): refactor and ut

* fix(iceberg): fix integration-test

typo

fix: fix ut

fix: fix test

address comments

address comments
@Li0k Li0k force-pushed the li0k/pick_82725fd1_to_dev_0808 branch from 16d4e35 to 3ce36e9 Compare September 26, 2025 06:54
@Li0k Li0k merged commit 3c63a85 into dev_rebase_main_20250808 Sep 26, 2025
15 of 21 checks passed
@Li0k Li0k deleted the li0k/pick_82725fd1_to_dev_0808 branch September 26, 2025 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants