Skip to content

Commit b91ee27

Browse files
committed
fix
1 parent c538452 commit b91ee27

File tree

4 files changed

+40
-68
lines changed

4 files changed

+40
-68
lines changed

src/paimon/fs/local/local_file.cpp

Lines changed: 10 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -123,69 +123,43 @@ Status LocalFile::Delete() const {
123123
return Status::OK();
124124
}
125125

126-
Status LocalFile::MkNestDir(const std::string& dir_name) const {
126+
Result<bool> LocalFile::MkNestDir(const std::string& dir_name) const {
127127
CHECK_HOOK();
128128
size_t pos = dir_name.rfind('/');
129129
if (pos == std::string::npos) {
130-
if (mkdir(dir_name.c_str(), 0755) < 0) {
131-
if (errno != EEXIST) {
132-
int32_t cur_errno = errno;
133-
return Status::IOError(fmt::format("MkNestDir path '{}' fail, ec: {}", path_,
134-
std::strerror(cur_errno)));
135-
}
136-
}
137-
return Status::OK();
130+
return mkdir(dir_name.c_str(), 0755) == 0;
138131
}
139132

140133
std::string parent_dir = dir_name.substr(0, pos);
141134
if (!parent_dir.empty() && access(parent_dir.c_str(), F_OK) != 0) {
142-
PAIMON_RETURN_NOT_OK(MkNestDir(parent_dir));
143-
}
144-
145-
if (mkdir(dir_name.c_str(), 0755) < 0) {
146-
if (errno != EEXIST) {
147-
int32_t cur_errno = errno;
148-
return Status::IOError(
149-
fmt::format("MkNestDir path '{}' fail, ec: {}", path_, std::strerror(cur_errno)));
135+
PAIMON_ASSIGN_OR_RAISE(bool success, MkNestDir(parent_dir));
136+
if (!success) {
137+
return false;
150138
}
151139
}
152-
return Status::OK();
140+
return mkdir(dir_name.c_str(), 0755) == 0;
153141
}
154142

155-
Status LocalFile::Mkdir() const {
143+
Result<bool> LocalFile::Mkdir() const {
156144
CHECK_HOOK();
157145
std::string dir = path_;
158146
size_t len = dir.size();
159147
if (dir[len - 1] == '/') {
160148
if (len == 1) {
161-
return Status::OK();
149+
return false;
162150
} else {
163151
dir.resize(len - 1);
164152
}
165153
}
166154
size_t pos = dir.rfind('/');
167155
if (pos == std::string::npos) {
168-
if (mkdir(dir.c_str(), 0755) < 0) {
169-
if (errno != EEXIST) {
170-
int32_t cur_errno = errno;
171-
return Status::IOError(
172-
fmt::format("Mkdir path '{}' fail, ec: {}", dir, std::strerror(cur_errno)));
173-
}
174-
}
175-
return Status::OK();
156+
return mkdir(dir.c_str(), 0755) == 0;
176157
}
177158
std::string parent_dir = dir.substr(0, pos);
178159
if (!parent_dir.empty() && access(parent_dir.c_str(), F_OK) != 0) {
179160
PAIMON_RETURN_NOT_OK(MkNestDir(parent_dir));
180161
}
181-
if (mkdir(dir.c_str(), 0755) < 0) {
182-
if (errno != EEXIST) {
183-
int32_t cur_errno = errno;
184-
return Status::IOError(
185-
fmt::format("create directory '{}' failed, ec: {}", dir, std::strerror(cur_errno)));
186-
}
187-
}
188-
return Status::OK();
162+
return mkdir(dir.c_str(), 0755) == 0;
189163
}
190164

191165
Result<std::unique_ptr<LocalFileStatus>> LocalFile::GetFileStatus() const {
@@ -350,13 +324,6 @@ Status LocalFile::OpenFile(bool is_read_file) {
350324
CHECK_HOOK();
351325
file_ = fopen(path_.c_str(), "r");
352326
} else {
353-
LocalFile parent_dir = GetParentFile();
354-
if (!parent_dir.GetAbsolutePath().empty()) {
355-
PAIMON_ASSIGN_OR_RAISE(bool is_exist, parent_dir.Exists());
356-
if (!is_exist) {
357-
PAIMON_RETURN_NOT_OK(parent_dir.Mkdir());
358-
}
359-
}
360327
CHECK_HOOK();
361328
file_ = fopen(path_.c_str(), "w");
362329
}

src/paimon/fs/local/local_file.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class LocalFile {
4747
Status Delete() const;
4848
const std::string& GetAbsolutePath() const;
4949
LocalFile GetParentFile() const;
50-
Status Mkdir() const;
50+
Result<bool> Mkdir() const;
5151
Result<std::unique_ptr<LocalFileStatus>> GetFileStatus() const;
5252
Result<uint64_t> Length() const;
5353
Result<int64_t> LastModifiedTimeMs() const;
@@ -68,7 +68,7 @@ class LocalFile {
6868
}
6969

7070
private:
71-
Status MkNestDir(const std::string& dir_name) const;
71+
Result<bool> MkNestDir(const std::string& dir_name) const;
7272

7373
const std::string path_;
7474
FILE* file_ = nullptr;

src/paimon/fs/local/local_file_system.cpp

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,16 @@ Status LocalFileSystem::MkdirsInternal(const LocalFile& file) const {
9090
}
9191
}
9292

93-
PAIMON_RETURN_NOT_OK(file.Mkdir());
93+
PAIMON_ASSIGN_OR_RAISE(bool success, file.Mkdir());
94+
if (!success) {
95+
PAIMON_ASSIGN_OR_RAISE(bool is_dir, file.IsDir());
96+
if (is_dir) {
97+
return Status::OK();
98+
} else {
99+
return Status::IOError(
100+
fmt::format("create directory '{}' failed", file.GetAbsolutePath()));
101+
}
102+
}
94103
return Status::OK();
95104
}
96105

@@ -210,17 +219,7 @@ Status LocalFileSystem::Rename(const std::string& src, const std::string& dst) c
210219
}
211220
PAIMON_ASSIGN_OR_RAISE(LocalFile dst_file, ToFile(dst));
212221
auto parent = dst_file.GetParentFile();
213-
if (!parent.GetAbsolutePath().empty()) {
214-
PAIMON_ASSIGN_OR_RAISE(bool is_exist, parent.Exists());
215-
if (is_exist) {
216-
// pass
217-
} else {
218-
Status status = parent.Mkdir();
219-
if (!status.ok() && !status.IsExist()) {
220-
return status;
221-
}
222-
}
223-
}
222+
PAIMON_RETURN_NOT_OK(Mkdirs(parent.GetAbsolutePath()));
224223
if (::rename(src.c_str(), dst.c_str()) != 0) {
225224
int32_t cur_errno = errno;
226225
return Status::IOError(err_msg, std::strerror(cur_errno));

src/paimon/fs/local/local_file_test.cpp

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ TEST(LocalFileTest, TestReadWriteEmptyContent) {
3333
if (dir.Exists().ok()) {
3434
ASSERT_TRUE(dir.Delete().ok());
3535
}
36-
ASSERT_TRUE(dir.Mkdir().ok());
36+
ASSERT_OK_AND_ASSIGN(bool success, dir.Mkdir());
37+
ASSERT_TRUE(success);
3738
std::string path = test_root + "/test.txt";
3839
LocalFile file = LocalFile(path);
3940
if (file.Exists().ok()) {
@@ -69,7 +70,8 @@ TEST(LocalFileTest, TestSimple) {
6970
if (dir.Exists().ok()) {
7071
ASSERT_OK(dir.Delete());
7172
}
72-
ASSERT_OK(dir.Mkdir());
73+
ASSERT_OK_AND_ASSIGN(bool success, dir.Mkdir());
74+
ASSERT_TRUE(success);
7375
std::string path = test_root + "/test.txt";
7476
LocalFile file = LocalFile(path);
7577
if (file.Exists().ok()) {
@@ -127,19 +129,24 @@ TEST(LocalFileTest, TestSimple) {
127129
ASSERT_EQ(strcmp(str_read, "test_data"), 0);
128130
}
129131

132+
ASSERT_OK_AND_ASSIGN(success, dir.Mkdir());
133+
ASSERT_FALSE(success);
134+
130135
ASSERT_OK(file2.Delete());
131136
ASSERT_FALSE(file2.Exists().value());
132137
}
133138

134139
TEST(LocalFileTest, TestUsage) {
135140
std::string test_root = "tmp/local_file_test_usage";
136141
LocalFile dir = LocalFile(test_root);
137-
ASSERT_OK(dir.Mkdir());
142+
ASSERT_OK_AND_ASSIGN(bool success, dir.Mkdir());
143+
ASSERT_TRUE(success);
138144
std::vector<std::string> file_list;
139145
ASSERT_OK(dir.List(&file_list));
140146
std::string path_deep_dir = test_root + "/tmp2/tmp3";
141147
LocalFile deep_dir = LocalFile(path_deep_dir);
142-
ASSERT_OK(deep_dir.Mkdir());
148+
ASSERT_OK_AND_ASSIGN(success, deep_dir.Mkdir());
149+
ASSERT_TRUE(success);
143150
LocalFile parent_deep_dir = deep_dir.GetParentFile();
144151
ASSERT_EQ(parent_deep_dir.GetAbsolutePath(), test_root + "/tmp2");
145152
ASSERT_OK(deep_dir.Delete());
@@ -155,7 +162,8 @@ TEST(LocalFileTest, TestOpenFile) {
155162
if (dir.Exists().ok()) {
156163
ASSERT_OK(dir.Delete());
157164
}
158-
ASSERT_OK(dir.Mkdir());
165+
ASSERT_OK_AND_ASSIGN(bool success, dir.Mkdir());
166+
ASSERT_TRUE(success);
159167
std::string path = test_root + "/test.txt";
160168
LocalFile file = LocalFile(path);
161169
if (file.Exists().ok()) {
@@ -167,20 +175,18 @@ TEST(LocalFileTest, TestOpenFile) {
167175
ASSERT_NOK_WITH_MSG(file.OpenFile(/*is_read_file=*/true), "file not exist");
168176
ASSERT_NOK_WITH_MSG(dir.OpenFile(/*is_read_file=*/true), "cannot open a directory");
169177

170-
std::string path2 = test_root + "/foo/test.txt";
171-
LocalFile file2 = LocalFile(path2);
172-
ASSERT_OK(file2.OpenFile(/*is_read_file=*/false));
173-
174178
std::string path3 = "test.txt";
175179
LocalFile file3 = LocalFile(path3);
176180
ASSERT_OK(file3.OpenFile(/*is_read_file=*/false));
177181
ASSERT_OK_AND_ASSIGN(int64_t modify_time, file3.LastModifiedTimeMs());
178182
ASSERT_GE(modify_time, -1);
179183

180184
LocalFile dir2 = LocalFile("/");
181-
ASSERT_OK(dir2.Mkdir());
185+
ASSERT_OK_AND_ASSIGN(success, dir2.Mkdir());
186+
ASSERT_FALSE(success);
182187
LocalFile dir3 = LocalFile(test_root + "/");
183-
ASSERT_OK(dir3.Mkdir());
188+
ASSERT_OK_AND_ASSIGN(success, dir3.Mkdir());
189+
ASSERT_FALSE(success);
184190
}
185191

186192
} // namespace paimon::test

0 commit comments

Comments
 (0)