Skip to content

feat(util): add location resolve and relativize helpers #1124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xndai wants to merge 1 commit into
base: main
Choose a base branch
from
+243 −0
Open

feat(util): add location resolve and relativize helpers #1124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 157 additions & 0 deletions src/iceberg/test/location_util_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,161 @@ TEST(LocationUtilTest, ParseScheme) {
#endif
}

TEST(LocationUtilTest, HasScheme) {
EXPECT_TRUE(LocationUtil::HasScheme("s3://bucket/db/table"));
EXPECT_TRUE(LocationUtil::HasScheme("hdfs://wh/db/table"));
EXPECT_TRUE(LocationUtil::HasScheme("file:/tmp/table"));
EXPECT_TRUE(LocationUtil::HasScheme("S3A://bucket/path"));
EXPECT_TRUE(LocationUtil::HasScheme("a+b-c.d1:rest"));
// Strict on every platform, unlike ParseScheme: a drive letter is a scheme.
EXPECT_TRUE(LocationUtil::HasScheme("C:/tmp/file.parquet"));
EXPECT_TRUE(LocationUtil::HasScheme("x:"));

EXPECT_FALSE(LocationUtil::HasScheme(""));
EXPECT_FALSE(LocationUtil::HasScheme("data/00000-0.parquet"));
EXPECT_FALSE(LocationUtil::HasScheme("/wh/db/table"));
EXPECT_FALSE(LocationUtil::HasScheme("://bucket/path"));
EXPECT_FALSE(LocationUtil::HasScheme(":"));
// The first character must be a letter.
EXPECT_FALSE(LocationUtil::HasScheme("1s3://bucket"));
EXPECT_FALSE(LocationUtil::HasScheme("+s3://bucket"));
// Characters outside the scheme grammar before the colon.
EXPECT_FALSE(LocationUtil::HasScheme("data/part:1.parquet"));
EXPECT_FALSE(LocationUtil::HasScheme("my_scheme://bucket"));
EXPECT_FALSE(LocationUtil::HasScheme("\\\\?\\C:\\long\\file.parquet"));
}

// Every example from the spec's "Path Resolution" table.
TEST(LocationUtilTest, ResolveLocationSpecExamples) {
// Relative path: parts are joined on `/`.
EXPECT_EQ(LocationUtil::ResolveLocation("s3://bucket/db/table", "data/00000-0.parquet"),
"s3://bucket/db/table/data/00000-0.parquet");
// Absolute path is used.
EXPECT_EQ(LocationUtil::ResolveLocation("s3://bucket/db/table",
"hdfs://wh/db/table/data/00000-0.parquet"),
"hdfs://wh/db/table/data/00000-0.parquet");
// Duplicate separator: the table location ends in `/`, which the spec
// recommends against. No second separator is added (see ResolveLocation).
EXPECT_EQ(
LocationUtil::ResolveLocation("s3://bucket/db/table/", "data/00000-0.parquet"),
"s3://bucket/db/table/data/00000-0.parquet");
// Duplicate separator: the relative path starts with `/`.
EXPECT_EQ(
LocationUtil::ResolveLocation("s3://bucket/db/table", "/data/00000-0.parquet"),
"s3://bucket/db/table//data/00000-0.parquet");
// Fully-qualified (v3 and earlier) path is used.
EXPECT_EQ(LocationUtil::ResolveLocation("s3://bucket/db/table",
"s3://bucket/db/table/data/00000-0.parquet"),
"s3://bucket/db/table/data/00000-0.parquet");
// Missing scheme (v3 and earlier): prepending a scheme to a pre-v4 path is
// not resolution and is not done here; such a path is not absolute.
EXPECT_FALSE(LocationUtil::HasScheme("/wh/db/table/data/00000-0.parquet"));
}

TEST(LocationUtilTest, ResolveLocation) {
// No `.` or `..` normalization.
EXPECT_EQ(LocationUtil::ResolveLocation("s3://b/t", "./data/f.parquet"),
"s3://b/t/./data/f.parquet");
EXPECT_EQ(LocationUtil::ResolveLocation("s3://b/t", "../other/f.parquet"),
"s3://b/t/../other/f.parquet");
// A trailing separator on the table location is not doubled; only one
// trailing separator is consumed, and a leading one on the path is kept.
EXPECT_EQ(LocationUtil::ResolveLocation("s3://b/t/", "f.parquet"),
"s3://b/t/f.parquet");
EXPECT_EQ(LocationUtil::ResolveLocation("s3://b/t//", "f.parquet"),
"s3://b/t//f.parquet");
EXPECT_EQ(LocationUtil::ResolveLocation("s3://b/t/", "/f.parquet"),
"s3://b/t//f.parquet");
// A scheme-less table location is joined the same way.
EXPECT_EQ(LocationUtil::ResolveLocation("/wh/t", "data/f.parquet"),
"/wh/t/data/f.parquet");
// A different scheme is absolute.
EXPECT_EQ(LocationUtil::ResolveLocation("s3://b/t", "gs://other/f.parquet"),
"gs://other/f.parquet");
// A colon after the first segment does not make a path absolute.
EXPECT_EQ(LocationUtil::ResolveLocation("s3://b/t", "data/a:b.parquet"),
"s3://b/t/data/a:b.parquet");
EXPECT_EQ(LocationUtil::ResolveLocation("s3://b/t", ""), "s3://b/t/");
}

TEST(LocationUtilTest, RelativizeLocation) {
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://bucket/db/table",
"s3://bucket/db/table/data/00000-0.parquet"),
"data/00000-0.parquet");
// Only one separator is removed.
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://bucket/db/table",
"s3://bucket/db/table//data/f.parquet"),
"/data/f.parquet");
// Outside the table location: unchanged.
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://bucket/db/table",
"s3://bucket/other/f.parquet"),
"s3://bucket/other/f.parquet");
// A sibling that shares the prefix is not under the table location.
EXPECT_EQ(LocationUtil::RelativizeLocation("/t", "/t2/data/f.parquet"),
"/t2/data/f.parquet");
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://b/t", "s3://b/t2/f.parquet"),
"s3://b/t2/f.parquet");
// The table location itself is not under it.
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://b/t", "s3://b/t"), "s3://b/t");
// A different scheme does not match.
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://b/t", "s3a://b/t/f.parquet"),
"s3a://b/t/f.parquet");
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://b/t", "gs://b/t/f.parquet"),
"gs://b/t/f.parquet");
// Matching is case-sensitive.
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://b/t", "S3://b/t/f.parquet"),
"S3://b/t/f.parquet");
// A table location ending in `/` supplies the separator itself.
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://b/t/", "s3://b/t/f.parquet"),
"f.parquet");
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://b/t/", "s3://b/t//f.parquet"),
"/f.parquet");
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://b/t/", "s3://b/t/"), "s3://b/t/");
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://b/t/", "s3://b/t2/f.parquet"),
"s3://b/t2/f.parquet");
// Already relative: unchanged.
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://b/t", "data/f.parquet"),
"data/f.parquet");
}

// Paths are opaque strings: percent-encoded, reserved and non-ASCII
// characters are copied as-is, never decoded or re-encoded.
TEST(LocationUtilTest, EscapedCharactersArePreserved) {
EXPECT_EQ(LocationUtil::ResolveLocation("s3://b/t", "data/a%20b%2Fc.parquet"),
"s3://b/t/data/a%20b%2Fc.parquet");
EXPECT_EQ(LocationUtil::ResolveLocation("s3://b/my%20t", "data/x=a b/f.parquet"),
"s3://b/my%20t/data/x=a b/f.parquet");
EXPECT_EQ(LocationUtil::ResolveLocation("s3://b/t", "data/k=v?#&+/f.parquet"),
"s3://b/t/data/k=v?#&+/f.parquet");
EXPECT_EQ(LocationUtil::ResolveLocation("s3://b/t", "data/\xc3\xa9t\xc3\xa9.parquet"),
"s3://b/t/data/\xc3\xa9t\xc3\xa9.parquet");

EXPECT_EQ(
LocationUtil::RelativizeLocation("s3://b/my%20t", "s3://b/my%20t/a%2Fb.parquet"),
"a%2Fb.parquet");
// An encoded and a decoded form are different paths: no match.
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://b/my%20t", "s3://b/my t/f.parquet"),
"s3://b/my t/f.parquet");
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://b/my t", "s3://b/my%20t/f.parquet"),
"s3://b/my%20t/f.parquet");
// An encoded separator is not a separator.
EXPECT_EQ(LocationUtil::RelativizeLocation("s3://b/t", "s3://b/t%2Ff.parquet"),
"s3://b/t%2Ff.parquet");
}

TEST(LocationUtilTest, RelativizeThenResolveRoundTrips) {
for (std::string_view table : {"s3://bucket/db/table", "s3://bucket/db/table/"}) {
for (std::string_view path :
{"s3://bucket/db/table/data/00000-0.parquet",
"s3://bucket/db/table/metadata/snap-1.parquet",
"s3://bucket/db/table//x.parquet", "s3://bucket/other/f.parquet",
"hdfs://wh/db/table/f.parquet"}) {
EXPECT_EQ(LocationUtil::ResolveLocation(
table, LocationUtil::RelativizeLocation(table, path)),
path)
<< "table: " << table;
}
}
}

} // namespace iceberg
59 changes: 59 additions & 0 deletions src/iceberg/util/location_util.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,63 @@ std::string_view LocationUtil::ParseScheme(std::string_view location) {
return candidate;
}

bool LocationUtil::HasScheme(std::string_view location) {
// RFC 3986 section 3.1: a URI starts with `scheme ":"`, so the scheme is
// everything before the first colon. The spec ("Paths in Metadata") calls a
// path absolute iff it starts with such a scheme, as Java's
// LocationUtil.hasScheme does.
const auto colon = location.find(':');
return colon != std::string_view::npos && IsValidScheme(location.substr(0, colon));
}

std::string LocationUtil::ResolveLocation(std::string_view table_location,
std::string_view location) {
if (HasScheme(location)) {
return std::string(location);
}
// The spec ("Path Resolution",
// https://iceberg.apache.org/spec/#path-resolution) defines this as a plain
// string join with `/`.
// It is not RFC 3986 reference resolution (section 5.2), which the spec
// rules out: a URI library would remove `.`/`..` segments, merge `//`, and
// could decode or re-encode `%XX`, producing a path that differs from the
// one the writer stored. Both parts are opaque strings in whatever encoding
// the writer used, and they are copied byte for byte.
//
// The spec recommends a table location that does not end in `/`, because

@zhjwpku zhjwpku Nov 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure this is the right solution, but from the spec I can see the trailing / is kept, I tend to align with spec[1], correct me if I'm wrong :-)

[1] https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/util/LocationUtil.java

@zhjwpku zhjwpku Nov 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Quote comment from java:

* The separator is appended unconditionally; {@code tableLocation} is expected
* not to end with {@code /} and {@code location} is expected not to start
* with {@code /}. Otherwise the result will contain a duplicate {@code //}.

@xndai xndai Nov 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The spec says exactly this:

The recommended convention for table location is to not end in a path separator because the join process would add a second separator character. (See example below.)

https://iceberg.apache.org/spec/#path-resolution

So it recommend not to include the trailing / for table location, but this diff go ahead and enforce that. It's very problematic for S3 if you include double back slash as they are not treated as file path. So s3://foo//bar is actually different then s3://foo/bar. Super confusing and error prone. That's why i just enforce that in the diff.

We could also add an assertion but I feel it's a bit too strong and there's an easy remedy.

// the join would otherwise produce `//`. This applies that recommendation:
// if the table location already ends in `/`, no second separator is added,
// since object stores such as S3 treat `//` as part of the key and advise
// against it. A leading `/` on the relative path is kept, since it comes
// from a stored path that really contains `//`.
std::string resolved;
resolved.reserve(table_location.size() + 1 + location.size());
resolved.append(table_location);
if (!table_location.ends_with('/')) {
resolved.push_back('/');
}
resolved.append(location);
return resolved;
}

std::string_view LocationUtil::RelativizeLocation(std::string_view table_location,
std::string_view location) {
// The spec ("Path Relativization") defines this as a plain prefix match on
// the table location followed by `/`, so a byte comparison is exact. No URI
// parsing or normalization: paths that only differ in encoding or `//` are
// different paths, and are kept absolute. A table location that already
// ends in `/` supplies the separator itself, matching ResolveLocation.
if (table_location.ends_with('/')) {
if (location.size() > table_location.size() && location.starts_with(table_location)) {
return location.substr(table_location.size());
}
return location;
}
if (location.size() > table_location.size() && location.starts_with(table_location) &&
location[table_location.size()] == '/') {
return location.substr(table_location.size() + 1);
}
return location;
}

} // namespace iceberg
27 changes: 27 additions & 0 deletions src/iceberg/util/location_util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
/// \file iceberg/util/location_util.h
/// \brief Provide location path utilities.

#include <string>
#include <string_view>

#include "iceberg/iceberg_export.h"
Expand All @@ -39,6 +40,32 @@ class ICEBERG_EXPORT LocationUtil {
/// not validated.
static std::string_view ParseScheme(std::string_view location);

/// \brief Whether a location starts with a URI scheme.
///
/// This is the spec's test for an absolute path (see "Paths in Metadata"):

@zhjwpku zhjwpku Nov 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we copy the descriptions from java impl?

@xndai xndai Nov 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

sure. but the difference needs to be called out. So will make this change after previous thread is resolved.

/// the text before the first colon must be a non-empty RFC 3986 scheme. It
/// is strict on every platform: unlike ParseScheme, a single letter such as
/// `C:` counts as a scheme.
static bool HasScheme(std::string_view location);

/// \brief Resolve a location against a table location.
///
/// A location with a URI scheme is absolute and is returned unchanged.
/// Otherwise the result is `table_location + "/" + location`, with no `/`
/// added when the table location already ends in one. The join does not
/// interpret `.` or `..`, and a location starting with `/` yields `//`.
static std::string ResolveLocation(std::string_view table_location,
std::string_view location);

/// \brief Relativize a location against a table location.
///
/// If the location starts with the table location immediately followed by
/// `/`, the remainder after that separator is returned. Otherwise the
/// location is returned unchanged. A table location that ends in `/`
/// already includes the separator.
static std::string_view RelativizeLocation(std::string_view table_location,
std::string_view location);

static std::string_view StripTrailingSlash(std::string_view path) {
if (path.empty()) {
return "";
Expand Down
Loading