yadavay-amzn commented on code in PR #818:
URL: https://github.com/apache/iceberg-cpp/pull/818#discussion_r3526610508
##########
src/iceberg/arrow/arrow_io.cc:
##########
@@ -484,24 +485,63 @@ class ArrowOutputFile : public OutputFile {
} // namespace
Result<std::string> ArrowFileSystemFileIO::ResolvePath(const std::string&
file_location) {
- const auto pos = file_location.find("://");
- if (pos == std::string::npos) {
- return file_location;
- }
-
- auto path = arrow_fs_->PathFromUri(file_location);
+ // Detect whether the location is a URI by looking for a scheme component.
+ // A URI scheme (RFC 3986 ยง3.1) starts with a letter and is followed by any
+ // combination of letters, digits, '+', '-', or '.', ending at the first ':'.
+ // A single character before ':' is a Windows drive letter (e.g., "C:\..."),
+ // not a URI scheme.
+ auto colon_pos = file_location.find(':');
+ bool is_uri =
+ colon_pos != std::string::npos && colon_pos > 1 &&
+ std::isalpha(static_cast<unsigned char>(file_location[0])) &&
+ std::all_of(file_location.begin(),
+ file_location.begin() +
static_cast<std::ptrdiff_t>(colon_pos),
+ [](char c) {
+ return std::isalpha(static_cast<unsigned char>(c)) ||
+ std::isdigit(static_cast<unsigned char>(c)) || c ==
'+' ||
+ c == '-' || c == '.';
+ });
Review Comment:
Good call - extracted the RFC 3986 scheme detection into a shared
header-only helper IsUriScheme(std::string_view) in src/iceberg/util/uri.h, and
both ResolvePath() here and DetectBuiltinFileIO() in rest_file_io.cc now call
it, so the logic is no longer duplicated.
##########
src/iceberg/catalog/rest/rest_file_io.cc:
##########
@@ -51,12 +53,25 @@ std::unordered_map<std::string, std::string>
MergeFileIOProperties(
} // namespace
Result<BuiltinFileIOKind> DetectBuiltinFileIO(std::string_view location) {
- const auto pos = location.find("://");
- if (pos == std::string_view::npos) {
+ // Detect URI scheme using RFC 3986 rules: a scheme is 2+ chars starting with
+ // a letter, followed by letters/digits/+/-/., ending at ':'.
+ // A single char before ':' is a Windows drive letter, not a scheme.
+ auto colon_pos = location.find(':');
+ bool is_uri =
+ colon_pos != std::string_view::npos && colon_pos > 1 &&
+ std::isalpha(static_cast<unsigned char>(location[0])) &&
+ std::all_of(location.begin(),
+ location.begin() + static_cast<std::ptrdiff_t>(colon_pos),
[](char c) {
+ return std::isalpha(static_cast<unsigned char>(c)) ||
+ std::isdigit(static_cast<unsigned char>(c)) || c ==
'+' ||
+ c == '-' || c == '.';
+ });
Review Comment:
Done - this and the arrow_io.cc site now both call the shared IsUriScheme()
helper in src/iceberg/util/uri.h; the duplicated detection block is removed.
--
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]
