(paimon) branch master updated: [core] Fix expire_snapshots accepting retain_min <= 0 and deleting files of retained snapshots (#10089)

[lzljs3620320]

This is an automated email from the ASF dual-hosted git repository.

JingsongLi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/paimon.git


The following commit(s) were added to refs/heads/master by this push:
     new 684cca2add [core] Fix expire_snapshots accepting retain_min <= 0 and 
deleting files of retained snapshots (#10089)
684cca2add is described below

commit 684cca2adde4f52d56500c0105e83e2f94ec74f8
Author: Li Guo <alexguo...@gmail.com>
AuthorDate: Wed Sep 23 23:39:53 2026 -0700

    [core] Fix expire_snapshots accepting retain_min <= 0 and deleting files of 
retained snapshots (#10089)
---
 .../apache/paimon/table/ExpireSnapshotsImpl.java   |  2 +
 .../paimon/operation/ExpireSnapshotsTest.java      | 44 ++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git 
a/paimon-core/src/main/java/org/apache/paimon/table/ExpireSnapshotsImpl.java 
b/paimon-core/src/main/java/org/apache/paimon/table/ExpireSnapshotsImpl.java
index 4b80085489..f06c12240b 100644
--- a/paimon-core/src/main/java/org/apache/paimon/table/ExpireSnapshotsImpl.java
+++ b/paimon-core/src/main/java/org/apache/paimon/table/ExpireSnapshotsImpl.java
@@ -130,6 +130,8 @@ public class ExpireSnapshotsImpl implements ExpireSnapshots 
{
             return 0;
         }
 
+        Preconditions.checkArgument(
+                retainMin >= 1, String.format("retainMin (%s) must be at least 
1.", retainMin));
         Preconditions.checkArgument(
                 retainMax >= retainMin,
                 String.format(
diff --git 
a/paimon-core/src/test/java/org/apache/paimon/operation/ExpireSnapshotsTest.java
 
b/paimon-core/src/test/java/org/apache/paimon/operation/ExpireSnapshotsTest.java
index 7fac929cdd..c0a113a90e 100644
--- 
a/paimon-core/src/test/java/org/apache/paimon/operation/ExpireSnapshotsTest.java
+++ 
b/paimon-core/src/test/java/org/apache/paimon/operation/ExpireSnapshotsTest.java
@@ -89,11 +89,13 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static java.util.Objects.requireNonNull;
 import static org.apache.paimon.data.BinaryRow.EMPTY_ROW;
 import static org.apache.paimon.utils.HintFileUtils.EARLIEST;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 /** Base test class for {@link ExpireSnapshotsImpl}. */
 public class ExpireSnapshotsTest {
@@ -738,6 +740,42 @@ public class ExpireSnapshotsTest {
         store.assertCleaned();
     }
 
+    @Test
+    public void testExpireRejectsNonPositiveRetainMin() throws Exception {
+        TestFileStore inputStore = 
createStore(CoreOptions.ChangelogProducer.INPUT);
+        SnapshotManager snapshotManager = inputStore.snapshotManager();
+
+        List<KeyValue> allData = new ArrayList<>();
+        List<Integer> snapshotPositions = new ArrayList<>();
+        commit(inputStore, 5, allData, snapshotPositions);
+        int latestSnapshotId = 
requireNonNull(snapshotManager.latestSnapshotId()).intValue();
+        for (int i = 1; i <= latestSnapshotId; i++) {
+            rewriteSnapshotTime(inputStore.fileIO(), snapshotManager, i, 0);
+        }
+        Set<java.nio.file.Path> filesBefore = listFiles();
+
+        for (int retainMin : new int[] {0, -1}) {
+            ExpireConfig config =
+                    ExpireConfig.builder()
+                            .snapshotRetainMin(retainMin)
+                            .snapshotRetainMax(Integer.MAX_VALUE)
+                            .snapshotTimeRetain(Duration.ZERO)
+                            .build();
+            ExpireSnapshots expire = inputStore.newExpire(config);
+            assertThatThrownBy(expire::expire)
+                    .isInstanceOf(IllegalArgumentException.class)
+                    .hasMessage("retainMin (" + retainMin + ") must be at 
least 1.");
+        }
+
+        // nothing may be deleted, every snapshot must still be readable
+        assertThat(listFiles()).isEqualTo(filesBefore);
+        for (int i = 1; i <= latestSnapshotId; i++) {
+            assertThat(snapshotManager.snapshotExists(i)).isTrue();
+            assertSnapshot(inputStore, i, allData, snapshotPositions);
+        }
+        inputStore.assertCleaned();
+    }
+
     @Test
     public void testExpireCollectsSnapshotsConcurrently() throws Exception {
         
store.options().toConfiguration().set(CoreOptions.FILE_OPERATION_THREAD_NUM, 4);
@@ -1498,6 +1536,12 @@ public class ExpireSnapshotsTest {
                 store.options().scanManifestParallelism());
     }
 
+    private Set<java.nio.file.Path> listFiles() throws IOException {
+        try (Stream<java.nio.file.Path> files = Files.walk(tempDir)) {
+            return 
files.filter(Files::isRegularFile).collect(Collectors.toSet());
+        }
+    }
+
     private void rewriteSnapshotTime(long snapshotId, long newTimeMillis) 
throws IOException {
         rewriteSnapshotTime(fileIO, snapshotManager, snapshotId, 
newTimeMillis);
     }