szehon-ho commented on code in PR #15173:
URL: https://github.com/apache/iceberg/pull/15173#discussion_r2738661968
##########
core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java:
##########
@@ -754,14 +759,33 @@ public static String fileName(String path) {
return filename;
}
- /** Relativize a path. */
+ /**
+ * Compute the relative path from a prefix to a given path.
+ *
+ * <p>If the path is under the prefix, returns the portion after the prefix.
If the path equals
+ * the prefix (representing the root directory itself), returns an empty
string.
+ *
+ * <p>Trailing separators are normalized: both "/a" and "/a/" are treated as
equivalent prefixes.
+ *
+ * @param path absolute path to relativize
+ * @param prefix prefix path to remove
+ * @return relative path from prefix to path, or empty string if path equals
prefix
+ * @throws IllegalArgumentException if path is not under or equal to prefix
+ */
public static String relativize(String path, String prefix) {
String toRemove = maybeAppendFileSeparator(prefix);
- if (!path.startsWith(toRemove)) {
- throw new IllegalArgumentException(
- String.format("Path %s does not start with %s", path, toRemove));
+
+ if (path.startsWith(toRemove)) {
+ return path.substring(toRemove.length());
}
- return path.substring(toRemove.length());
+
+ // Handle exact match where path equals prefix (without trailing separator)
+ if (maybeAppendFileSeparator(path).equals(toRemove)) {
Review Comment:
question: would it also work to just have an extra condition in the initial
code and not have to change the rest of the code? something like:
ie, if (! (path.startsWith(toRemove) || path.equals(toRemove))
##########
core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java:
##########
@@ -754,14 +759,33 @@ public static String fileName(String path) {
return filename;
}
- /** Relativize a path. */
+ /**
+ * Compute the relative path from a prefix to a given path.
+ *
+ * <p>If the path is under the prefix, returns the portion after the prefix.
If the path equals
+ * the prefix (representing the root directory itself), returns an empty
string.
+ *
+ * <p>Trailing separators are normalized: both "/a" and "/a/" are treated as
equivalent prefixes.
+ *
+ * @param path absolute path to relativize
+ * @param prefix prefix path to remove
+ * @return relative path from prefix to path, or empty string if path equals
prefix
+ * @throws IllegalArgumentException if path is not under or equal to prefix
+ */
public static String relativize(String path, String prefix) {
String toRemove = maybeAppendFileSeparator(prefix);
- if (!path.startsWith(toRemove)) {
- throw new IllegalArgumentException(
- String.format("Path %s does not start with %s", path, toRemove));
+
+ if (path.startsWith(toRemove)) {
Review Comment:
question: reading the next line (maybeAppendFileSepator(path)), why dont we
need to do it here as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
