lucasfang commented on code in PR #338:
URL: https://github.com/apache/paimon-cpp/pull/338#discussion_r4005901718
##########
src/paimon/fs/jindo/jindo_file_system.cpp:
##########
@@ -173,11 +173,18 @@ Result<FileStatus> JindoFileSystem::GetFileStatus(const
std::string& path) const
Status JindoFileSystem::ListDir(const std::string& directory,
std::vector<BasicFileStatus>*
file_status_list) const {
- PAIMON_ASSIGN_OR_RAISE(bool exist, Exists(directory));
- if (!exist) {
- return Status::OK();
+ // One status call answers what Exists() followed by GetFileStatus() asked
the store twice:
+ // whether the path is there at all, and whether it is a directory.
PAIMON_RETURN_NOT_OK_FROM_
+ // JINDO maps the SDK's not-found to Status::NotExist, which is what tells
a missing directory
+ // (listed as empty, as the other file systems do) from a call that
genuinely failed.
+ Result<FileStatus> dir_status = GetFileStatus(directory);
+ if (!dir_status.ok()) {
+ if (dir_status.status().IsNotExist()) {
+ return Status::OK();
+ }
+ return dir_status.status();
Review Comment:
confirmed on both halves. The mapping is covered by
JindoUtilsTest.TestMacroMapsStatusByErrorCode, which asserts
JDO_FILE_NOT_FOUND_ERROR becomes Status::NotExist (and other codes become
IOError). For the SDK side there was no UT, so I added the network-gated
JindoFileSystemTest.TestMissingPathReportsNotExistAndMissingDirListsEmpty:
against real OSS it asserts GetFileStatus on a missing path fails with
IsNotExist(), and ListDir on a missing directory returns OK with an empty list.
It passes locally (Jindo suites 13/13).
--
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]
