From e64c68badb7065d04157f177b604dd368b0bd707 Mon Sep 17 00:00:00 2001 From: Socrates Date: Mon, 16 Mar 2026 16:44:18 +0800 Subject: [PATCH 1/2] [fix](iceberg) Fix execute action validation gaps --- .../action/IcebergRewriteDataFilesAction.java | 3 ++ .../IcebergRollbackToTimestampAction.java | 21 +++++++++++- .../test_iceberg_execute_actions.groovy | 34 ++++++++++++++++++- 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/action/IcebergRewriteDataFilesAction.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/action/IcebergRewriteDataFilesAction.java index 885f15225b0ff0..eb34eab0217d64 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/action/IcebergRewriteDataFilesAction.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/action/IcebergRewriteDataFilesAction.java @@ -161,6 +161,9 @@ protected void validateIcebergAction() throws UserException { if (this.maxFileSizeBytes == 0) { this.maxFileSizeBytes = (long) (targetFileSizeBytes * 1.8); } + if (this.minFileSizeBytes > this.maxFileSizeBytes) { + throw new UserException("min-file-size-bytes must be less than or equal to max-file-size-bytes"); + } validateNoPartitions(); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/action/IcebergRollbackToTimestampAction.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/action/IcebergRollbackToTimestampAction.java index be83f57ed13319..f01367a85dc896 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/action/IcebergRollbackToTimestampAction.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/action/IcebergRollbackToTimestampAction.java @@ -36,6 +36,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.TimeZone; /** * Iceberg rollback to timestamp action implementation. @@ -103,7 +104,7 @@ protected List executeAction(TableIf table) throws UserException { Long previousSnapshotId = previousSnapshot != null ? previousSnapshot.snapshotId() : null; try { - long targetTimestamp = TimeUtils.msTimeStringToLong(timestampStr, TimeUtils.getTimeZone()); + long targetTimestamp = parseTimestampMillis(timestampStr, TimeUtils.getTimeZone()); icebergTable.manageSnapshots().rollbackToTime(targetTimestamp).commit(); Snapshot currentSnapshot = icebergTable.currentSnapshot(); @@ -133,4 +134,22 @@ protected List getResultSchema() { public String getDescription() { return "Rollback Iceberg table to the snapshot that was current at a specific timestamp"; } + + static long parseTimestampMillis(String timestampStr, TimeZone timeZone) { + String trimmed = timestampStr.trim(); + try { + long timestampMs = Long.parseLong(trimmed); + if (timestampMs < 0) { + throw new IllegalArgumentException("Timestamp must be non-negative: " + timestampMs); + } + return timestampMs; + } catch (NumberFormatException e) { + long parsedTimestamp = TimeUtils.msTimeStringToLong(trimmed, timeZone); + if (parsedTimestamp < 0) { + throw new IllegalArgumentException("Invalid timestamp format. Expected ISO datetime " + + "(yyyy-MM-dd HH:mm:ss.SSS) or timestamp in milliseconds: " + trimmed, e); + } + return parsedTimestamp; + } + } } diff --git a/regression-test/suites/external_table_p0/iceberg/action/test_iceberg_execute_actions.groovy b/regression-test/suites/external_table_p0/iceberg/action/test_iceberg_execute_actions.groovy index b235fbbd23b0cc..8aef659db18b45 100644 --- a/regression-test/suites/external_table_p0/iceberg/action/test_iceberg_execute_actions.groovy +++ b/regression-test/suites/external_table_p0/iceberg/action/test_iceberg_execute_actions.groovy @@ -264,6 +264,25 @@ suite("test_iceberg_optimize_actions_ddl", "p0,external") { logger.info("Rollback timestamp result: ${rollbackTimestampResult}") qt_after_rollback_to_timestamp """SELECT * FROM test_rollback_timestamp ORDER BY id""" + String middleCommittedTime = timestampSnapshotList[1][0] + LocalDateTime middleDateTime = LocalDateTime.parse(middleCommittedTime, unifiedFormatter) + String epochMillisSnapshotTime = String.valueOf( + middleDateTime.atZone(ZoneId.systemDefault()).toInstant().toEpochMilli()) + + List> rollbackTimestampEpochResult = sql """ + ALTER TABLE ${catalog_name}.${db_name}.test_rollback_timestamp + EXECUTE rollback_to_timestamp("timestamp" = "${epochMillisSnapshotTime}") + """ + logger.info("Rollback epoch millis result: ${rollbackTimestampEpochResult}") + + List> rowsAfterEpochRollback = sql """ + SELECT id, version FROM test_rollback_timestamp ORDER BY id + """ + assertTrue(rowsAfterEpochRollback.size() == 2, + "Expected rollback_to_timestamp with epoch millis to keep exactly 2 rows") + assertTrue(rowsAfterEpochRollback[0][0] == 1 && rowsAfterEpochRollback[1][0] == 2, + "Expected rollback_to_timestamp with epoch millis to restore the first two snapshots") + // ===================================================================================== // Test Case 3: set_current_snapshot action @@ -484,6 +503,19 @@ suite("test_iceberg_optimize_actions_ddl", "p0,external") { exception "Invalid target-file-size-bytes format: not-a-number" } + // Test rewrite_data_files with invalid min/max file size relationship + test { + sql """ + ALTER TABLE ${catalog_name}.${db_name}.${table_name} EXECUTE rewrite_data_files + ( + "target-file-size-bytes" = "536870912", + "min-file-size-bytes" = "1073741824", + "max-file-size-bytes" = "536870912" + ) + """ + exception "min-file-size-bytes must be less than or equal to max-file-size-bytes" + } + // Test set_current_snapshot with both snapshot_id and ref test { sql """ @@ -631,4 +663,4 @@ test { } -} \ No newline at end of file +} From 30531d85a0f39f8afc4fed43975dc46918e64580 Mon Sep 17 00:00:00 2001 From: Socrates Date: Tue, 17 Mar 2026 16:19:51 +0800 Subject: [PATCH 2/2] [test](iceberg) Fix rollback timestamp regression case --- .../iceberg/action/test_iceberg_execute_actions.groovy | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/regression-test/suites/external_table_p0/iceberg/action/test_iceberg_execute_actions.groovy b/regression-test/suites/external_table_p0/iceberg/action/test_iceberg_execute_actions.groovy index 8aef659db18b45..a4bcd1dd419a81 100644 --- a/regression-test/suites/external_table_p0/iceberg/action/test_iceberg_execute_actions.groovy +++ b/regression-test/suites/external_table_p0/iceberg/action/test_iceberg_execute_actions.groovy @@ -264,10 +264,8 @@ suite("test_iceberg_optimize_actions_ddl", "p0,external") { logger.info("Rollback timestamp result: ${rollbackTimestampResult}") qt_after_rollback_to_timestamp """SELECT * FROM test_rollback_timestamp ORDER BY id""" - String middleCommittedTime = timestampSnapshotList[1][0] - LocalDateTime middleDateTime = LocalDateTime.parse(middleCommittedTime, unifiedFormatter) String epochMillisSnapshotTime = String.valueOf( - middleDateTime.atZone(ZoneId.systemDefault()).toInstant().toEpochMilli()) + dateTime.atZone(ZoneId.systemDefault()).toInstant().toEpochMilli()) List> rollbackTimestampEpochResult = sql """ ALTER TABLE ${catalog_name}.${db_name}.test_rollback_timestamp