github-actions[bot] commented on code in PR #31214:
URL: https://github.com/apache/doris/pull/31214#discussion_r1497259323
##########
be/src/io/fs/local_file_system.cpp:
##########
@@ -199,42 +199,85 @@ Status LocalFileSystem::directory_size(const Path&
dir_path, size_t* dir_size) {
return Status::InternalError("faile to get dir size {}",
dir_path.native());
}
-Status LocalFileSystem::list_impl(const Path& dir, bool only_file,
std::vector<FileInfo>* files,
- bool* exists) {
- RETURN_IF_ERROR(exists_impl(dir, exists));
- if (!exists) {
- return Status::OK();
+class LocalFileListIterator final : public FileListIterator {
+public:
+ LocalFileListIterator(const Path& dir, bool only_file) : dir(dir),
only_file(only_file) {}
+ ~LocalFileListIterator() override = default;
+
+ Status init() {
Review Comment:
warning: method 'init' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status init() {
```
##########
be/src/io/fs/hdfs_file_system.cpp:
##########
@@ -261,7 +262,36 @@ Status HdfsFileSystem::file_size_impl(const Path& path,
int64_t* file_size) cons
return Status::OK();
}
-Status HdfsFileSystem::list_impl(const Path& path, bool only_file,
std::vector<FileInfo>* files,
+struct HdfsFileListIterator final : public FileListIterator {
+public:
+ HdfsFileListIterator(int numEntries, hdfsFileInfo* hdfs_file_info, bool
only_file)
+ : numEntries(numEntries), hdfs_file_info(hdfs_file_info),
only_file(only_file) {}
+ ~HdfsFileListIterator() override { hdfsFreeFileInfo(hdfs_file_info,
numEntries); }
Review Comment:
warning: use '= default' to define a trivial destructor
[modernize-use-equals-default]
```cpp
~HdfsFileListIterator() override { hdfsFreeFileInfo(hdfs_file_info,
numEntries); }
^
```
##########
be/src/io/fs/s3_file_system.cpp:
##########
@@ -312,7 +312,71 @@ Status S3FileSystem::file_size_impl(const Path& file,
int64_t* file_size) const
return Status::OK();
}
-Status S3FileSystem::list_impl(const Path& dir, bool only_file,
std::vector<FileInfo>* files,
+class S3FileListIterator final : public FileListIterator {
+public:
+ S3FileListIterator(std::shared_ptr<Aws::S3::S3Client> client,
+ Aws::S3::Model::ListObjectsV2Request request, bool
only_file,
+ std::string_view prefix, std::string full_path)
+ : client(std::move(client)),
+ request(std::move(request)),
+ only_file(only_file),
+ prefix(prefix),
+ full_path(std::move(full_path)) {}
+ ~S3FileListIterator() override = default;
+
+ bool has_next() const override {
+ return !is_trucated && iter != outcome.GetResult().GetContents().end();
+ }
+ Result<FileInfo> next() override {
+ FileInfo file_info;
+ while (!is_trucated && iter !=
outcome.GetResult().GetContents().end()) {
+ for (; iter != outcome.GetResult().GetContents().end(); iter++) {
+ const auto& obj = *iter;
+ std::string key = obj.GetKey();
+ bool is_dir = (key.back() == '/');
+ if (only_file && is_dir) {
+ continue;
+ }
+ file_info.file_name = obj.GetKey().substr(prefix.size());
+ file_info.file_size = obj.GetSize();
+ file_info.is_file = !is_dir;
+ return file_info;
+ }
+ if (auto st = get_list_outcome(); !st.ok()) [[unlikely]] {
+ return ResultError(std::move(st));
+ }
+ }
+ return ResultError(Status::InternalError("Unable to iterate"));
+ }
+
+private:
+ Status get_list_outcome() {
Review Comment:
warning: method 'get_list_outcome' can be made const
[readability-make-member-function-const]
```suggestion
Status get_list_outcome() const {
```
--
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
