Skip to content

Core: Relative paths for data files#16666

Open
rambleraptor wants to merge 1 commit into
apache:mainfrom
rambleraptor:relative-paths-2
Open

Core: Relative paths for data files#16666
rambleraptor wants to merge 1 commit into
apache:mainfrom
rambleraptor:relative-paths-2

Conversation

@rambleraptor

@rambleraptor rambleraptor commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

This adds support for reading data files using relative paths.

We've had some issues in the prior PR where a bunch of files have scheme-less "absolute" paths. By Iceberg's definition of absolute, those need schemes. I fixed the ones we ran into.

@rambleraptor rambleraptor marked this pull request as draft June 2, 2026 17:53
@rambleraptor rambleraptor changed the title Relative paths for data files Core: Relative paths for data files Jun 2, 2026
@github-actions github-actions Bot added the core label Jun 2, 2026
@github-actions github-actions Bot added the spark label Jun 2, 2026
@rambleraptor rambleraptor force-pushed the relative-paths-2 branch 2 times, most recently from 1e16f03 to 4484754 Compare June 3, 2026 00:20
@github-actions github-actions Bot added the data label Jun 3, 2026
@rambleraptor rambleraptor marked this pull request as ready for review June 3, 2026 17:31
@rambleraptor

Copy link
Copy Markdown
Contributor Author

@talatuyarer @danielcweeks as the relative paths gurus, please take a look!

@anoopj I know you've been doing work in the space as well

this.manifestLocation = manifestLocation;
}

void setLocation(String location) {

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.

This is an open design question, but I think BaseFile is probably not the correct place to inject the location since we want to isolate this to v4-only code paths. Most likely, we will do the path resolution/relativization in the v4-only adapter layer, as done in this prototype.

file.setFileSequenceNumber(manifestEntry.fileSequenceNumber());
file.setManifestLocation(manifestLocation);
if (baseLocation != null && file.path() != null) {
file.setLocation(LocationUtil.resolveLocation(baseLocation, file.location()));

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.

I'm not certain, but I suspect this might cause manifest rewrite paths to always persist the absolute location. (Appends paths would be fine). I guess the tests are probably passing because the v3 paths are always absolute.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants