diff --git a/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java b/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java index f250d2e12289..44ccf2fe4098 100644 --- a/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java +++ b/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java @@ -18,7 +18,6 @@ */ package org.apache.iceberg; -import java.io.File; import java.io.IOException; import java.io.Serializable; import java.io.UncheckedIOException; @@ -51,6 +50,9 @@ public class RewriteTablePathUtil { private static final Logger LOG = LoggerFactory.getLogger(RewriteTablePathUtil.class); + // Use the POSIX separator instead of File.separator because File.separator is dependent on + // the client environment and not the target filesystem. POSIX is compatible with S3, GCS, etc + public static final String FILE_SEPARATOR = "/"; private RewriteTablePathUtil() {} @@ -534,18 +536,13 @@ public static String newPath(String path, String sourcePrefix, String targetPref /** Combine a base and relative path. */ public static String combinePaths(String absolutePath, String relativePath) { - String combined = absolutePath; - if (!combined.endsWith("/")) { - combined += "/"; - } - combined += relativePath; - return combined; + return maybeAppendFileSeparator(absolutePath) + relativePath; } /** Returns the file name of a path. */ public static String fileName(String path) { String filename = path; - int lastIndex = path.lastIndexOf(File.separator); + int lastIndex = path.lastIndexOf(FILE_SEPARATOR); if (lastIndex != -1) { filename = path.substring(lastIndex + 1); } @@ -554,10 +551,7 @@ public static String fileName(String path) { /** Relativize a path. */ public static String relativize(String path, String prefix) { - String toRemove = prefix; - if (!toRemove.endsWith("/")) { - toRemove += "/"; - } + String toRemove = maybeAppendFileSeparator(prefix); if (!path.startsWith(toRemove)) { throw new IllegalArgumentException( String.format("Path %s does not start with %s", path, toRemove)); @@ -565,6 +559,10 @@ public static String relativize(String path, String prefix) { return path.substring(toRemove.length()); } + public static String maybeAppendFileSeparator(String path) { + return path.endsWith(FILE_SEPARATOR) ? path : path + FILE_SEPARATOR; + } + /** * Construct a staging path under a given staging directory * diff --git a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/RewriteTablePathSparkAction.java b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/RewriteTablePathSparkAction.java index 64469384b881..9aef3b74c680 100644 --- a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/RewriteTablePathSparkAction.java +++ b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/RewriteTablePathSparkAction.java @@ -18,7 +18,6 @@ */ package org.apache.iceberg.spark.actions; -import java.io.File; import java.io.IOException; import java.util.List; import java.util.Map; @@ -180,9 +179,13 @@ private void validateInputs() { validateAndSetStartVersion(); if (stagingDir == null) { - stagingDir = getMetadataLocation(table) + "copy-table-staging-" + UUID.randomUUID() + "/"; - } else if (!stagingDir.endsWith("/")) { - stagingDir = stagingDir + "/"; + stagingDir = + getMetadataLocation(table) + + "copy-table-staging-" + + UUID.randomUUID() + + RewriteTablePathUtil.FILE_SEPARATOR; + } else { + stagingDir = RewriteTablePathUtil.maybeAppendFileSeparator(stagingDir); } } @@ -361,7 +364,8 @@ private Pair rewriteVersionFile(TableMetadata metadata, String v TableMetadata newTableMetadata = RewriteTablePathUtil.replacePaths(metadata, sourcePrefix, targetPrefix); TableMetadataParser.overwrite(newTableMetadata, table.io().newOutputFile(stagingPath)); - return Pair.of(stagingPath, newPath(versionFilePath, sourcePrefix, targetPrefix)); + return Pair.of( + stagingPath, RewriteTablePathUtil.newPath(versionFilePath, sourcePrefix, targetPrefix)); } /** @@ -392,7 +396,9 @@ private RewriteResult rewriteManifestList( result.append(rewriteResult); // add the manifest list copy plan itself to the result - result.copyPlan().add(Pair.of(outputPath, newPath(path, sourcePrefix, targetPrefix))); + result + .copyPlan() + .add(Pair.of(outputPath, RewriteTablePathUtil.newPath(path, sourcePrefix, targetPrefix))); return result; } @@ -710,15 +716,10 @@ private boolean fileExist(String path) { return table.io().newInputFile(path).exists(); } - private static String newPath(String path, String sourcePrefix, String targetPrefix) { - return RewriteTablePathUtil.combinePaths( - targetPrefix, RewriteTablePathUtil.relativize(path, sourcePrefix)); - } - private String getMetadataLocation(Table tbl) { String currentMetadataPath = ((HasTableOperations) tbl).operations().current().metadataFileLocation(); - int lastIndex = currentMetadataPath.lastIndexOf(File.separator); + int lastIndex = currentMetadataPath.lastIndexOf(RewriteTablePathUtil.FILE_SEPARATOR); String metadataDir = ""; if (lastIndex != -1) { metadataDir = currentMetadataPath.substring(0, lastIndex + 1);