github-actions[bot] commented on code in PR #17586:
URL: https://github.com/apache/doris/pull/17586#discussion_r1133249764
##########
be/src/io/fs/local_file_reader.h:
##########
@@ -47,6 +42,9 @@ class LocalFileReader final : public FileReader {
FileSystemSPtr fs() const override { return _fs; }
+private:
+ Status read_at_impl(size_t offset, Slice result, size_t* bytes_read, const
IOContext* io_ctx) override;
+
private:
Review Comment:
warning: redundant access specifier has the same accessibility as the
previous access specifier [readability-redundant-access-specifiers]
```suggestion
```
**be/src/io/fs/local_file_reader.h:44:** previously declared here
```cpp
private:
^
```
##########
be/src/io/fs/local_file_system.cpp:
##########
@@ -34,211 +34,143 @@ LocalFileSystem::LocalFileSystem(Path&& root_path,
std::string&& id)
LocalFileSystem::~LocalFileSystem() = default;
-Path LocalFileSystem::absolute_path(const Path& path) const {
- if (path.is_absolute()) {
- return path;
- }
- return _root_path / path;
-}
-
-Status LocalFileSystem::create_file(const Path& path, FileWriterPtr* writer) {
- if (bthread_self() == 0) {
- return create_file_impl(path, writer);
- }
- Status s;
- auto task = [&] { s = create_file_impl(path, writer); };
- AsyncIO::run_task(task, io::FileSystemType::LOCAL);
- return s;
-}
-
-Status LocalFileSystem::create_file_impl(const Path& path, FileWriterPtr*
writer) {
- auto fs_path = absolute_path(path);
- int fd = ::open(fs_path.c_str(), O_TRUNC | O_WRONLY | O_CREAT | O_CLOEXEC,
0666);
+Status LocalFileSystem::create_file_impl(const Path& file, FileWriterPtr*
writer) {
+ int fd = ::open(file.c_str(), O_TRUNC | O_WRONLY | O_CREAT | O_CLOEXEC,
0666);
if (-1 == fd) {
- return Status::IOError("cannot open {}: {}", fs_path.native(),
std::strerror(errno));
+ return Status::IOError("cannot open {}: {}", file.native(),
std::strerror(errno));
}
*writer = std::make_unique<LocalFileWriter>(
- std::move(fs_path), fd,
std::static_pointer_cast<LocalFileSystem>(shared_from_this()));
+ std::move(file), fd,
std::static_pointer_cast<LocalFileSystem>(shared_from_this()));
Review Comment:
warning: std::move of the const variable 'file' has no effect; remove
std::move() or make the variable non-const [performance-move-const-arg]
```suggestion
file, fd,
std::static_pointer_cast<LocalFileSystem>(shared_from_this()));
```
##########
be/src/io/fs/local_file_system.cpp:
##########
@@ -34,211 +34,143 @@
LocalFileSystem::~LocalFileSystem() = default;
-Path LocalFileSystem::absolute_path(const Path& path) const {
- if (path.is_absolute()) {
- return path;
- }
- return _root_path / path;
-}
-
-Status LocalFileSystem::create_file(const Path& path, FileWriterPtr* writer) {
- if (bthread_self() == 0) {
- return create_file_impl(path, writer);
- }
- Status s;
- auto task = [&] { s = create_file_impl(path, writer); };
- AsyncIO::run_task(task, io::FileSystemType::LOCAL);
- return s;
-}
-
-Status LocalFileSystem::create_file_impl(const Path& path, FileWriterPtr*
writer) {
- auto fs_path = absolute_path(path);
- int fd = ::open(fs_path.c_str(), O_TRUNC | O_WRONLY | O_CREAT | O_CLOEXEC,
0666);
+Status LocalFileSystem::create_file_impl(const Path& file, FileWriterPtr*
writer) {
+ int fd = ::open(file.c_str(), O_TRUNC | O_WRONLY | O_CREAT | O_CLOEXEC,
0666);
if (-1 == fd) {
- return Status::IOError("cannot open {}: {}", fs_path.native(),
std::strerror(errno));
+ return Status::IOError("cannot open {}: {}", file.native(),
std::strerror(errno));
}
*writer = std::make_unique<LocalFileWriter>(
- std::move(fs_path), fd,
std::static_pointer_cast<LocalFileSystem>(shared_from_this()));
+ std::move(file), fd,
std::static_pointer_cast<LocalFileSystem>(shared_from_this()));
return Status::OK();
}
-Status LocalFileSystem::open_file(const Path& path, FileReaderSPtr* reader,
IOContext* io_ctx) {
- if (bthread_self() == 0) {
- return open_file_impl(path, reader, io_ctx);
- }
- Status s;
- auto task = [&] { s = open_file_impl(path, reader, io_ctx); };
- AsyncIO::run_task(task, io::FileSystemType::LOCAL);
- return s;
-}
-
-Status LocalFileSystem::open_file_impl(const Path& path, FileReaderSPtr*
reader,
- IOContext* /*io_ctx*/) {
- auto fs_path = absolute_path(path);
+Status LocalFileSystem::open_file_impl(const Path& file, const
FileReaderOptions& /*reader_options*/,
+ FileReaderSPtr* reader) {
size_t fsize = 0;
- RETURN_IF_ERROR(file_size(fs_path, &fsize));
+ RETURN_IF_ERROR(file_size_impl(file, &fsize));
int fd = -1;
- RETRY_ON_EINTR(fd, open(fs_path.c_str(), O_RDONLY));
+ RETRY_ON_EINTR(fd, open(file.c_str(), O_RDONLY));
if (fd < 0) {
- return Status::IOError("cannot open {}: {}", fs_path.native(),
std::strerror(errno));
+ return Status::IOError("cannot open {}: {}", file.native(),
std::strerror(errno));
}
*reader = std::make_shared<LocalFileReader>(
- std::move(fs_path), fsize, fd,
+ std::move(file), fsize, fd,
Review Comment:
warning: std::move of the const variable 'file' has no effect; remove
std::move() or make the variable non-const [performance-move-const-arg]
```suggestion
file, fsize, fd,
```
--
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]
