justfortaste commented on PR #43642:
URL: https://github.com/apache/doris/pull/43642#issuecomment-2562138665
> It is a huge pr, we should review carefully.
>
> There is some low level change, like abs_path =
std::filesystem::path(fmt::format("s3://{}/{}", _bucket, _prefix)) / path;
>
> I am not sure its impact.
RemoteFileSystem::download will call absolute_path two times
```
RemoteFileSystem::download(const Path& remote_file, const Path& local) {
S3FileSystem::absolute_path // first ime
S3FileSystem::download_impl
FileSystem::file_size
S3FileSystem::absolute_path // second time
```
when "path" is not absolue, the result will return with double "_prefix"
original code:
```
Status absolute_path(const Path& path, Path& abs_path) const override {
if (path.string().find("://") != std::string::npos) {
// the path is with schema, which means this is a full path like:
// s3://bucket/path/to/file.txt
// so no need to concat with prefix
abs_path = path;
} else {
// path with no schema
abs_path = _prefix / path;
}
return Status::OK();
}
```
This function is only effect with S3 file system, the path will be parsed
by "S3URI::parse" to get the key.
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
