Core: Extend TestRemoveSnapshots parameterization to V3 - #17772
Open
dkranchii wants to merge 1 commit into
Open
Conversation
Add format version 3 to the @parameters matrix so every test in TestRemoveSnapshots runs against V1, V2, and V3 with both cleanup strategies. Widen testExpireWithDeleteFiles to accept both V2 and V3 using the fileADeletes() / fileBDeletes() helpers from TestBase, which transparently return position-delete parquet files for V2 and deletion vectors for V3. V1 has no delete files and V4+ folds delete entries into the root manifest, so V2 and V3 are the only versions that carry standalone delete manifests.
3 tasks
uros-b
approved these changes
Aug 23, 2026
Member
|
LGTM, thank you @dkranchii! |
dramaticlly
approved these changes
Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extend
TestRemoveSnapshotsto run every test against format version 3 in addition to V1 and V2, and maketestExpireWithDeleteFilesversion-agnostic so it exercises both V2 position-delete parquet files and V3 deletion vectors.Motivation
Follow-up to reviewer feedback on #17745 asking for expanded V3 coverage in
TestRemoveSnapshots. V3 has been the default table format for new writes in recent releases but the snapshot-expiration test class only exercised V1 and V2. This closes that gap for the whole class, not just the delete-file test.Changes
@Parametersmatrix gains{3, true}and{3, false}entries, so every@TestTemplatein the class runs against V3 for both the incremental and reachable cleanup strategies.testExpireWithDeleteFilesnow uses the existingfileADeletes()/fileBDeletes()helpers fromTestBase(which return position deletes for V2 and DVs for V3) instead of hardcodedFILE_A_DELETES/FILE_B_DELETES. Version guard widened fromisEqualTo(2)toisIn(2, 3).V1 has no delete files and V4+ folds delete entries into the root manifest, so V2 and V3 remain the only versions that carry standalone delete manifests.
Test plan
./gradlew :iceberg-core:test --tests "org.apache.iceberg.TestRemoveSnapshots"passes for all six parameter combinations (V1/V2/V3 × incremental/reachable).AI Disclosure