Skip to content

Core: Treat path prefixes as literals in RewriteTablePathUtil.replacePaths - #17521

Open
uros-b wants to merge 1 commit into
apache:mainfrom
uros-b:core-rewritetablepath-literal-prefix
Open

Core: Treat path prefixes as literals in RewriteTablePathUtil.replacePaths#17521
uros-b wants to merge 1 commit into
apache:mainfrom
uros-b:core-rewritetablepath-literal-prefix

Conversation

@uros-b

@uros-b uros-b commented Aug 4, 2026

Copy link
Copy Markdown
Member

RewriteTablePathUtil.replacePaths rewrote the table location with
String.replaceFirst(sourcePrefix, targetPrefix), which interprets the user-supplied
sourcePrefix as a regular expression and targetPrefix as a replacement string.
Both are file paths, so this misbehaves whenever a path contains regex metacharacters:

  • sourcePrefix = s3://bucket/warehouse.db/table also matches
    s3://bucket/warehouseXdb/table, rewriting a location that is not under the prefix.
  • A sourcePrefix with a trailing separator (e.g. s3://bucket/warehouse/table/) does not
    match at all, so the location is silently left pointing at the source while every
    other path is rewritten to the target.
  • A targetPrefix containing $ fails with IndexOutOfBoundsException: No group 1, and a
    path containing an unbalanced [ fails with PatternSyntaxException.

Every other path rewrite in this class already goes through newPath() -> relativize(),
which compares prefixes literally (startsWith + substring), normalizes trailing
separators, and raises a clear IllegalArgumentException when a path is not under the
source prefix. In RewriteTablePathSparkAction#rewriteVersionFile the immediately
preceding stagingPath(...) call already applies that literal check, so this change makes
the location rewrite consistent with the validation already performed one line earlier.

Both prefixes are user-supplied through the public
RewriteTablePath.rewriteLocationPrefix(sourcePrefix, targetPrefix) API.

Tested with TestRewriteTablePathUtil (core) and TestRewriteTablePathsAction on Spark
3.5, 4.0 and 4.1.

Note: #14355 makes the same one-line change as a side effect of adding multiple
source/destination prefixes. This is the minimal standalone fix plus regression tests for
the metacharacter and trailing-separator cases, so it can land independently; whichever
merges first, the other should rebase cleanly on this method.

@github-actions github-actions Bot added the core label Aug 4, 2026

@uros-b uros-b left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I came across this while working on #17484, cc @krisnaru who's currently working on #14355

@uros-b

uros-b commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

cc @szehon-ho

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant