From 16a790942984ebf2f79d862b404c904d78e54773 Mon Sep 17 00:00:00 2001 From: uros-b <221401595+uros-b@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:20:23 +0000 Subject: [PATCH 1/2] Core: Treat path prefixes as literals in RewriteTablePathUtil.replacePaths --- .../apache/iceberg/RewriteTablePathUtil.java | 2 +- .../iceberg/TestRewriteTablePathUtil.java | 41 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java b/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java index e72dbef6dbcc..21b3b279423e 100644 --- a/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java +++ b/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java @@ -107,7 +107,7 @@ public Set> copyPlan() { */ public static TableMetadata replacePaths( TableMetadata metadata, String sourcePrefix, String targetPrefix) { - String newLocation = metadata.location().replaceFirst(sourcePrefix, targetPrefix); + String newLocation = newPath(metadata.location(), sourcePrefix, targetPrefix); List newSnapshots = updatePathInSnapshots(metadata, sourcePrefix, targetPrefix); List metadataLogEntries = updatePathInMetadataLogs(metadata, sourcePrefix, targetPrefix); diff --git a/core/src/test/java/org/apache/iceberg/TestRewriteTablePathUtil.java b/core/src/test/java/org/apache/iceberg/TestRewriteTablePathUtil.java index 1b0f5f6b1c70..8bc4c6d891c9 100644 --- a/core/src/test/java/org/apache/iceberg/TestRewriteTablePathUtil.java +++ b/core/src/test/java/org/apache/iceberg/TestRewriteTablePathUtil.java @@ -197,6 +197,47 @@ public void testNewPathBackupRestore() { .isEqualTo("/table"); } + @Test + public void testReplacePathsTreatsPrefixAsLiteral() { + // Prefixes are literal paths, not regular expressions. A prefix containing regex + // metacharacters must match only itself. + String sourcePrefix = "s3://bucket/warehouse.db/table"; + String targetPrefix = "s3://bucket/restored.db/table"; + TableMetadata metadata = + TableMetadata.newTableMetadata( + SCHEMA, PartitionSpec.unpartitioned(), sourcePrefix, ImmutableMap.of()); + + TableMetadata replaced = + RewriteTablePathUtil.replacePaths(metadata, sourcePrefix, targetPrefix); + assertThat(replaced.location()).isEqualTo(targetPrefix); + + // The '.' in the source prefix must not match an arbitrary character, so a location that + // only matches when the prefix is read as a regex is rejected. + TableMetadata unrelated = + TableMetadata.newTableMetadata( + SCHEMA, + PartitionSpec.unpartitioned(), + "s3://bucket/warehouseXdb/table", + ImmutableMap.of()); + assertThatThrownBy( + () -> RewriteTablePathUtil.replacePaths(unrelated, sourcePrefix, targetPrefix)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("does not start with"); + } + + @Test + public void testReplacePathsWithTrailingSeparatorInPrefix() { + // A source prefix with a trailing separator still matches the table location. + String location = "s3://bucket/warehouse/table"; + TableMetadata metadata = + TableMetadata.newTableMetadata( + SCHEMA, PartitionSpec.unpartitioned(), location, ImmutableMap.of()); + + TableMetadata replaced = + RewriteTablePathUtil.replacePaths(metadata, location + "/", "s3://bucket/restored/table"); + assertThat(replaced.location()).isEqualTo("s3://bucket/restored/table"); + } + @Test public void testNewPathTableRename() { // Rename /tableX to /table (target is substring of source name) From 43c313e483a1f64ad5cf2ffdc752e99473b3d693 Mon Sep 17 00:00:00 2001 From: uros-b <221401595+uros-b@users.noreply.github.com> Date: Tue, 18 Aug 2026 14:52:24 +0000 Subject: [PATCH 2/2] Address review --- .../iceberg/TestRewriteTablePathUtil.java | 66 ++++++++++++++++--- 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/core/src/test/java/org/apache/iceberg/TestRewriteTablePathUtil.java b/core/src/test/java/org/apache/iceberg/TestRewriteTablePathUtil.java index 8bc4c6d891c9..f4cb24c6b736 100644 --- a/core/src/test/java/org/apache/iceberg/TestRewriteTablePathUtil.java +++ b/core/src/test/java/org/apache/iceberg/TestRewriteTablePathUtil.java @@ -199,32 +199,80 @@ public void testNewPathBackupRestore() { @Test public void testReplacePathsTreatsPrefixAsLiteral() { - // Prefixes are literal paths, not regular expressions. A prefix containing regex - // metacharacters must match only itself. + // The '.' in the prefix is a literal character, not a regex wildcard; a location matching the + // prefix literally is rewritten to the target. String sourcePrefix = "s3://bucket/warehouse.db/table"; String targetPrefix = "s3://bucket/restored.db/table"; TableMetadata metadata = TableMetadata.newTableMetadata( SCHEMA, PartitionSpec.unpartitioned(), sourcePrefix, ImmutableMap.of()); - TableMetadata replaced = - RewriteTablePathUtil.replacePaths(metadata, sourcePrefix, targetPrefix); - assertThat(replaced.location()).isEqualTo(targetPrefix); + assertThat(RewriteTablePathUtil.replacePaths(metadata, sourcePrefix, targetPrefix).location()) + .isEqualTo(targetPrefix); + } - // The '.' in the source prefix must not match an arbitrary character, so a location that - // only matches when the prefix is read as a regex is rejected. - TableMetadata unrelated = + @Test + public void testReplacePathsRejectsRegexOnlyPrefixMatch() { + // Read as a regex, "warehouse.db" would match "warehouseXdb" ('.' matches 'X'). As a literal + // prefix it must not, so a location that only matches under regex semantics is rejected. + String sourcePrefix = "s3://bucket/warehouse.db/table"; + String targetPrefix = "s3://bucket/restored.db/table"; + TableMetadata metadata = TableMetadata.newTableMetadata( SCHEMA, PartitionSpec.unpartitioned(), "s3://bucket/warehouseXdb/table", ImmutableMap.of()); + assertThatThrownBy( - () -> RewriteTablePathUtil.replacePaths(unrelated, sourcePrefix, targetPrefix)) + () -> RewriteTablePathUtil.replacePaths(metadata, sourcePrefix, targetPrefix)) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("does not start with"); } + @Test + public void testReplacePathsTargetWithDollarSign() { + // '$1' in the target is a literal path segment, not a regex replacement group reference + // (which previously threw IndexOutOfBoundsException: No group 1). + String sourcePrefix = "s3://bucket/db/table"; + String targetPrefix = "s3://bucket/cost$1/table"; + TableMetadata metadata = + TableMetadata.newTableMetadata( + SCHEMA, PartitionSpec.unpartitioned(), sourcePrefix, ImmutableMap.of()); + + assertThat(RewriteTablePathUtil.replacePaths(metadata, sourcePrefix, targetPrefix).location()) + .isEqualTo(targetPrefix); + } + + @Test + public void testReplacePathsSourceWithUnbalancedBracket() { + // An unbalanced '[' in the prefix would be an invalid regex (PatternSyntaxException); as a + // literal it matches itself. + String sourcePrefix = "s3://bucket/db/table[0"; + String targetPrefix = "s3://bucket/db/restored"; + TableMetadata metadata = + TableMetadata.newTableMetadata( + SCHEMA, PartitionSpec.unpartitioned(), sourcePrefix, ImmutableMap.of()); + + assertThat(RewriteTablePathUtil.replacePaths(metadata, sourcePrefix, targetPrefix).location()) + .isEqualTo(targetPrefix); + } + + @Test + public void testReplacePathsSourceWithCharacterClass() { + // '[0]' is a valid regex character class matching '0', so as a regex it silently missed the + // real directory "table[0]" (while matching an unrelated "table0"); as a literal prefix it + // must match "table[0]". + String sourcePrefix = "s3://bucket/db/table[0]"; + String targetPrefix = "s3://bucket/db/restored"; + TableMetadata metadata = + TableMetadata.newTableMetadata( + SCHEMA, PartitionSpec.unpartitioned(), sourcePrefix, ImmutableMap.of()); + + assertThat(RewriteTablePathUtil.replacePaths(metadata, sourcePrefix, targetPrefix).location()) + .isEqualTo(targetPrefix); + } + @Test public void testReplacePathsWithTrailingSeparatorInPrefix() { // A source prefix with a trailing separator still matches the table location.