Skip to content

Spark: Fix DeleteOrphanFilesSparkAction sibling path matching bug - #16999

Open
CoderNasim2023 wants to merge 1 commit into
apache:mainfrom
CoderNasim2023:fix-orphan-files-scope
Open

Spark: Fix DeleteOrphanFilesSparkAction sibling path matching bug#16999
CoderNasim2023 wants to merge 1 commit into
apache:mainfrom
CoderNasim2023:fix-orphan-files-scope

Conversation

@CoderNasim2023

Copy link
Copy Markdown

**This commit fixes issue #16493 where file_list_view was scoped using raw string prefix matching, which allowed sibling paths to fall inside orphan cleanup (e.g. s3://bucket/table-backup matched s3://bucket/table).

Added a trailing slash to the location before prefix matching in filteredCompareToFileList to ensure precise directory scoping.**


AI Disclosure

  • Model: Gemini 2.5 Pro / Claude Sonnet 4.6
  • Platform/Tool: Antigravity IDE
  • Human Oversight: fully reviewed
  • Prompt Summary: Fix DeleteOrphanFilesSparkAction sibling path matching bug

This commit fixes issue apache#16493 where file_list_view was scoped using raw
string prefix matching, which allowed sibling paths to fall inside orphan
cleanup (e.g. s3://bucket/table-backup matched s3://bucket/table).

Added a trailing slash to the location before prefix matching in
filteredCompareToFileList to ensure precise directory scoping.
@github-actions github-actions Bot added the spark label Jun 29, 2026
Dataset<Row> files = compareToFileList;
if (location != null) {
files = files.filter(files.col(FILE_PATH).startsWith(location));
String strippedLocation = LocationUtil.stripTrailingSlash(location);

@RussellSpitzer RussellSpitzer Jun 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why don't we just always add the separator here? What does the check below get us?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

    LocationUtil.stripTrailingSlash(location) + LocationUtil.PATH_SEPARATOR;

@nssalian

nssalian commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

[#16498][(https://github.com//pull/16498) seems to be solving the same issue. @CoderNasim2023 you should coordinate with @wombatu-kun to see how to get this through in a singular effort rather than duplicating the work.

@wombatu-kun

Copy link
Copy Markdown
Contributor

Welcome, and thanks for tackling this, @CoderNasim2023!

This bug is already being fixed in #16498 (open since May 21), with the same approach - normalizing location to directory form (stripTrailingSlash(location) + "/") before the startsWith filter, across all three Spark trees. #16498 additionally clarifies the DeleteOrphanFiles.location javadoc and adds a dedicated test that also covers the path-equals-location edge case, and it already has a review from @sungwy.

To avoid splitting the effort, would you be up for reviewing or testing #16498 instead? That would help it land. I'll leave it to the maintainers to decide whether to close this one as a duplicate.

@CoderNasim2023

CoderNasim2023 commented Jul 4, 2026

Copy link
Copy Markdown
Author

Okk Mr @wombatu-kun please do response on this That would help . I'll leave it to the maintainers to decide whether to close this one and if its solve dthe issue then please ro merge it @RussellSpitzer

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.

4 participants