From 56664fbf48e445e4d9154078571e22e14a09e4f5 Mon Sep 17 00:00:00 2001 From: xuzifu666 <1206332514@qq.com> Date: Sun, 8 Sep 2024 14:58:24 +0800 Subject: [PATCH 1/7] [core] Manifest table query throw exception when scan.snapshot-id not exist to reminder with range of snapshot --- .../paimon/table/system/ManifestsTable.java | 17 ++++++---- .../paimon/flink/CatalogTableITCase.java | 32 +++++++++++++++++++ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java b/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java index 6184dbdad6ac..2df4c40a0b23 100644 --- a/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java +++ b/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java @@ -40,11 +40,7 @@ import org.apache.paimon.types.BigIntType; import org.apache.paimon.types.DataField; import org.apache.paimon.types.RowType; -import org.apache.paimon.utils.FileStorePathFactory; -import org.apache.paimon.utils.IteratorRecordReader; -import org.apache.paimon.utils.ProjectedRow; -import org.apache.paimon.utils.SerializationUtils; -import org.apache.paimon.utils.SnapshotManager; +import org.apache.paimon.utils.*; import org.apache.paimon.shade.guava30.com.google.common.collect.Iterators; @@ -195,7 +191,16 @@ private static List allManifests(FileStoreTable dataTable) { SnapshotManager snapshotManager = dataTable.snapshotManager(); Long snapshotId = coreOptions.scanSnapshotId(); Snapshot snapshot = null; - if (snapshotId != null && snapshotManager.snapshotExists(snapshotId)) { + if (snapshotId != null) { + // reminder user with snapshot id range + if (!snapshotManager.snapshotExists(snapshotId)) { + Long earliestSnapshotId = snapshotManager.earliestSnapshotId(); + Long latestSnapshotId = snapshotManager.latestSnapshotId(); + throw new RuntimeException( + String.format( + "scan.snapshot-id is not exist, you can set it in range from %s to %s", + earliestSnapshotId, latestSnapshotId)); + } snapshot = snapshotManager.snapshot(snapshotId); } else if (snapshotId == null) { snapshot = snapshotManager.latestSnapshot(); diff --git a/paimon-flink/paimon-flink-common/src/test/java/org/apache/paimon/flink/CatalogTableITCase.java b/paimon-flink/paimon-flink-common/src/test/java/org/apache/paimon/flink/CatalogTableITCase.java index ff4563004fb4..3aabb0fb016e 100644 --- a/paimon-flink/paimon-flink-common/src/test/java/org/apache/paimon/flink/CatalogTableITCase.java +++ b/paimon-flink/paimon-flink-common/src/test/java/org/apache/paimon/flink/CatalogTableITCase.java @@ -32,6 +32,7 @@ import org.apache.flink.table.catalog.exceptions.PartitionNotExistException; import org.apache.flink.table.catalog.exceptions.TableNotPartitionedException; import org.apache.flink.types.Row; +import org.assertj.core.api.AssertionsForInterfaceTypes; import org.junit.jupiter.api.Test; import javax.annotation.Nonnull; @@ -105,6 +106,37 @@ public void testSnapshotsTable() throws Exception { assertThat(result).contains(Row.of(1L, 0L, "APPEND"), Row.of(2L, 0L, "APPEND")); } + @Test + public void testManifestsTableWithSnapshotId() { + sql(String.format("CREATE TABLE T (id INT PRIMARY KEY NOT ENFORCED, name STRING)")); + + sql("INSERT INTO T VALUES (1, '111111111'), (2, '2'), (3, '3'), (4, '4')"); + + sql("INSERT INTO T VALUES (2, '2_1'), (3, '3_1')"); + + sql("INSERT INTO T VALUES (2, '2_2'), (4, '4_1')"); + + AssertionsForInterfaceTypes.assertThat(batchSql("SELECT * FROM T")) + .containsExactlyInAnyOrder( + Row.of(1, "111111111"), + Row.of(2, "2_2"), + Row.of(3, "3_1"), + Row.of(4, "4_1")); + List result = + sql( + "SELECT num_added_files, num_deleted_files, schema_id FROM T$manifests /*+ OPTIONS('scan.snapshot-id'='2') */"); + + assertThat(result).containsExactlyInAnyOrder(Row.of(1L, 0L, 0L), Row.of(1L, 0L, 0L)); + + assertThatThrownBy( + () -> + sql( + "SELECT num_added_files, num_deleted_files, schema_id FROM T$manifests /*+ OPTIONS('scan.snapshot-id'='6') */")) + .hasCauseInstanceOf(RuntimeException.class) + .hasRootCauseMessage( + "scan.snapshot-id is not exist, you can set it in range from 1 to 3"); + } + @Test public void testSnapshotsTableWithRecordCount() throws Exception { sql("CREATE TABLE T (a INT, b INT)"); From c9c32498dea63f2cda6ac8a1d22b825e13af877e Mon Sep 17 00:00:00 2001 From: xuzifu666 <1206332514@qq.com> Date: Sun, 8 Sep 2024 18:24:30 +0800 Subject: [PATCH 2/7] fix import --- .../java/org/apache/paimon/table/system/ManifestsTable.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java b/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java index 2df4c40a0b23..d078486488c6 100644 --- a/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java +++ b/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java @@ -40,7 +40,11 @@ import org.apache.paimon.types.BigIntType; import org.apache.paimon.types.DataField; import org.apache.paimon.types.RowType; -import org.apache.paimon.utils.*; +import org.apache.paimon.utils.FileStorePathFactory; +import org.apache.paimon.utils.IteratorRecordReader; +import org.apache.paimon.utils.ProjectedRow; +import org.apache.paimon.utils.SerializationUtils; +import org.apache.paimon.utils.SnapshotManager; import org.apache.paimon.shade.guava30.com.google.common.collect.Iterators; From fb699c6482077b9ed2f3a65d461b818f2a89c80f Mon Sep 17 00:00:00 2001 From: xuzifu666 <1206332514@qq.com> Date: Sun, 8 Sep 2024 19:41:31 +0800 Subject: [PATCH 3/7] fix ut --- .../table/system/ManifestsTableTest.java | 6 ++-- .../paimon/flink/CatalogTableITCase.java | 32 ------------------- 2 files changed, 3 insertions(+), 35 deletions(-) diff --git a/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java b/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java index edca0831d168..a0145eab982c 100644 --- a/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java +++ b/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java @@ -45,7 +45,8 @@ import java.util.List; import static org.apache.paimon.utils.FileStorePathFactoryTest.createNonPartFactory; -import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.*; +import static org.junit.Assert.assertThrows; /** Unit tests for {@link ManifestsTable}. */ public class ManifestsTableTest extends TableTestBase { @@ -118,8 +119,7 @@ public void testReadManifestsFromNotExistSnapshot() throws Exception { (ManifestsTable) manifestsTable.copy( Collections.singletonMap(CoreOptions.SCAN_SNAPSHOT_ID.key(), "3")); - List result = read(manifestsTable); - assertThat(result).isEmpty(); + assertThrows(RuntimeException.class, () -> read(manifestsTable)); } private List getExpectedResult(long snapshotId) { diff --git a/paimon-flink/paimon-flink-common/src/test/java/org/apache/paimon/flink/CatalogTableITCase.java b/paimon-flink/paimon-flink-common/src/test/java/org/apache/paimon/flink/CatalogTableITCase.java index 3aabb0fb016e..ff4563004fb4 100644 --- a/paimon-flink/paimon-flink-common/src/test/java/org/apache/paimon/flink/CatalogTableITCase.java +++ b/paimon-flink/paimon-flink-common/src/test/java/org/apache/paimon/flink/CatalogTableITCase.java @@ -32,7 +32,6 @@ import org.apache.flink.table.catalog.exceptions.PartitionNotExistException; import org.apache.flink.table.catalog.exceptions.TableNotPartitionedException; import org.apache.flink.types.Row; -import org.assertj.core.api.AssertionsForInterfaceTypes; import org.junit.jupiter.api.Test; import javax.annotation.Nonnull; @@ -106,37 +105,6 @@ public void testSnapshotsTable() throws Exception { assertThat(result).contains(Row.of(1L, 0L, "APPEND"), Row.of(2L, 0L, "APPEND")); } - @Test - public void testManifestsTableWithSnapshotId() { - sql(String.format("CREATE TABLE T (id INT PRIMARY KEY NOT ENFORCED, name STRING)")); - - sql("INSERT INTO T VALUES (1, '111111111'), (2, '2'), (3, '3'), (4, '4')"); - - sql("INSERT INTO T VALUES (2, '2_1'), (3, '3_1')"); - - sql("INSERT INTO T VALUES (2, '2_2'), (4, '4_1')"); - - AssertionsForInterfaceTypes.assertThat(batchSql("SELECT * FROM T")) - .containsExactlyInAnyOrder( - Row.of(1, "111111111"), - Row.of(2, "2_2"), - Row.of(3, "3_1"), - Row.of(4, "4_1")); - List result = - sql( - "SELECT num_added_files, num_deleted_files, schema_id FROM T$manifests /*+ OPTIONS('scan.snapshot-id'='2') */"); - - assertThat(result).containsExactlyInAnyOrder(Row.of(1L, 0L, 0L), Row.of(1L, 0L, 0L)); - - assertThatThrownBy( - () -> - sql( - "SELECT num_added_files, num_deleted_files, schema_id FROM T$manifests /*+ OPTIONS('scan.snapshot-id'='6') */")) - .hasCauseInstanceOf(RuntimeException.class) - .hasRootCauseMessage( - "scan.snapshot-id is not exist, you can set it in range from 1 to 3"); - } - @Test public void testSnapshotsTableWithRecordCount() throws Exception { sql("CREATE TABLE T (a INT, b INT)"); From 73cc353c5590aea4980af599966eff11e528b226 Mon Sep 17 00:00:00 2001 From: xuzifu666 <1206332514@qq.com> Date: Sun, 8 Sep 2024 19:48:14 +0800 Subject: [PATCH 4/7] fix ut --- .../java/org/apache/paimon/table/system/ManifestsTableTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java b/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java index a0145eab982c..f1c216729e4e 100644 --- a/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java +++ b/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java @@ -45,7 +45,7 @@ import java.util.List; import static org.apache.paimon.utils.FileStorePathFactoryTest.createNonPartFactory; -import static org.assertj.core.api.Assertions.*; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertThrows; /** Unit tests for {@link ManifestsTable}. */ From c4860582a1f0fc123441ad96c9118f33b5911301 Mon Sep 17 00:00:00 2001 From: xuzifu666 <1206332514@qq.com> Date: Sun, 8 Sep 2024 19:52:27 +0800 Subject: [PATCH 5/7] fix msg --- .../java/org/apache/paimon/table/system/ManifestsTable.java | 4 ++-- .../org/apache/paimon/table/system/ManifestsTableTest.java | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java b/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java index d078486488c6..ba1d82ecbf94 100644 --- a/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java +++ b/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java @@ -202,8 +202,8 @@ private static List allManifests(FileStoreTable dataTable) { Long latestSnapshotId = snapshotManager.latestSnapshotId(); throw new RuntimeException( String.format( - "scan.snapshot-id is not exist, you can set it in range from %s to %s", - earliestSnapshotId, latestSnapshotId)); + "Specified scan.snapshot-id %s is not exist, you can set it in range from %s to %s", + snapshotId, earliestSnapshotId, latestSnapshotId)); } snapshot = snapshotManager.snapshot(snapshotId); } else if (snapshotId == null) { diff --git a/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java b/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java index f1c216729e4e..4aee65c7169d 100644 --- a/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java +++ b/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java @@ -119,7 +119,10 @@ public void testReadManifestsFromNotExistSnapshot() throws Exception { (ManifestsTable) manifestsTable.copy( Collections.singletonMap(CoreOptions.SCAN_SNAPSHOT_ID.key(), "3")); - assertThrows(RuntimeException.class, () -> read(manifestsTable)); + assertThrows( + "Specified scan.snapshot-id 3 is not exist, you can set it in range from 1 to 2", + RuntimeException.class, + () -> read(manifestsTable)); } private List getExpectedResult(long snapshotId) { From aa97befba1a5e770918dbf41312fd1a3fff411dd Mon Sep 17 00:00:00 2001 From: xuzifu666 <1206332514@qq.com> Date: Sun, 8 Sep 2024 20:26:33 +0800 Subject: [PATCH 6/7] fix ex type --- .../java/org/apache/paimon/table/system/ManifestsTable.java | 3 ++- .../org/apache/paimon/table/system/ManifestsTableTest.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java b/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java index ba1d82ecbf94..0c7d36d5eaac 100644 --- a/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java +++ b/paimon-core/src/main/java/org/apache/paimon/table/system/ManifestsTable.java @@ -45,6 +45,7 @@ import org.apache.paimon.utils.ProjectedRow; import org.apache.paimon.utils.SerializationUtils; import org.apache.paimon.utils.SnapshotManager; +import org.apache.paimon.utils.SnapshotNotExistException; import org.apache.paimon.shade.guava30.com.google.common.collect.Iterators; @@ -200,7 +201,7 @@ private static List allManifests(FileStoreTable dataTable) { if (!snapshotManager.snapshotExists(snapshotId)) { Long earliestSnapshotId = snapshotManager.earliestSnapshotId(); Long latestSnapshotId = snapshotManager.latestSnapshotId(); - throw new RuntimeException( + throw new SnapshotNotExistException( String.format( "Specified scan.snapshot-id %s is not exist, you can set it in range from %s to %s", snapshotId, earliestSnapshotId, latestSnapshotId)); diff --git a/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java b/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java index 4aee65c7169d..ca449f16fadb 100644 --- a/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java +++ b/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java @@ -37,6 +37,7 @@ import org.apache.paimon.types.DataTypes; import org.apache.paimon.utils.SnapshotManager; +import org.apache.paimon.utils.SnapshotNotExistException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -121,7 +122,7 @@ public void testReadManifestsFromNotExistSnapshot() throws Exception { Collections.singletonMap(CoreOptions.SCAN_SNAPSHOT_ID.key(), "3")); assertThrows( "Specified scan.snapshot-id 3 is not exist, you can set it in range from 1 to 2", - RuntimeException.class, + SnapshotNotExistException.class, () -> read(manifestsTable)); } From bac7ace95678e8b44c92d5d552f766bd716739ca Mon Sep 17 00:00:00 2001 From: xuzifu666 <1206332514@qq.com> Date: Sun, 8 Sep 2024 20:30:52 +0800 Subject: [PATCH 7/7] fix type --- .../java/org/apache/paimon/table/system/ManifestsTableTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java b/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java index ca449f16fadb..32dd0d24888d 100644 --- a/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java +++ b/paimon-core/src/test/java/org/apache/paimon/table/system/ManifestsTableTest.java @@ -36,8 +36,8 @@ import org.apache.paimon.table.TableTestBase; import org.apache.paimon.types.DataTypes; import org.apache.paimon.utils.SnapshotManager; - import org.apache.paimon.utils.SnapshotNotExistException; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test;