Skip to content

Commit ff446f9

Browse files
authored
fix(fs): create parent directories before opening Jindo writers (#430)
1 parent a4c1fef commit ff446f9

4 files changed

Lines changed: 154 additions & 1 deletion

File tree

src/paimon/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -796,7 +796,8 @@ if(PAIMON_BUILD_TESTS)
796796
# OSS access and stay disabled.
797797
set(FS_TEST_JINDO_SOURCES)
798798
if(PAIMON_ENABLE_JINDO)
799-
list(APPEND FS_TEST_JINDO_SOURCES fs/jindo/jindo_utils_test.cpp)
799+
list(APPEND FS_TEST_JINDO_SOURCES fs/jindo/jindo_file_system_unit_test.cpp
800+
fs/jindo/jindo_utils_test.cpp)
800801
endif()
801802

802803
add_paimon_test(fs_test

src/paimon/fs/jindo/jindo_file_system.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "fmt/format.h"
2727
#include "jdo_error.h" // NOLINT(build/include_subdir)
2828
#include "paimon/common/utils/math.h"
29+
#include "paimon/common/utils/path_util.h"
2930
#include "paimon/fs/jindo/jindo_file_status.h"
3031
#include "paimon/fs/jindo/jindo_utils.h"
3132

@@ -68,6 +69,18 @@ Result<std::unique_ptr<OutputStream>> JindoFileSystem::Create(const std::string&
6869
return Status::Invalid(
6970
fmt::format("do not allow overwrite, but the file {} already exists", path));
7071
}
72+
const std::string parent_path = PathUtil::GetParentDirPath(path);
73+
if (!parent_path.empty()) {
74+
PAIMON_ASSIGN_OR_RAISE(Path parent, PathUtil::ToPath(parent_path));
75+
// Do not issue mkdir for scheme-only or authority-only URI parents.
76+
if (!parent.path.empty()) {
77+
PAIMON_RETURN_NOT_OK(Mkdirs(parent_path));
78+
}
79+
}
80+
return OpenWriter(path);
81+
}
82+
83+
Result<std::unique_ptr<OutputStream>> JindoFileSystem::OpenWriter(const std::string& path) const {
7184
std::unique_ptr<JdoWriter> writer;
7285
PAIMON_RETURN_NOT_OK_FROM_JINDO(impl_->GetFileSystem()->openWriter(path, &writer));
7386
return std::make_unique<JindoOutputStream>(impl_, std::move(writer));

src/paimon/fs/jindo/jindo_file_system.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ class JindoFileSystem : public FileSystem {
5757

5858
Result<bool> Exists(const std::string& path) const override;
5959

60+
protected:
61+
virtual Result<std::unique_ptr<OutputStream>> OpenWriter(const std::string& path) const;
62+
6063
private:
6164
std::shared_ptr<JindoFileSystemImpl> impl_;
6265
};
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
/*
2+
* Copyright 2026-present Alibaba Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#include <cstdint>
18+
#include <memory>
19+
#include <string>
20+
#include <utility>
21+
#include <vector>
22+
23+
#include "gtest/gtest.h"
24+
#include "paimon/fs/jindo/jindo_file_system.h"
25+
#include "paimon/testing/utils/testharness.h"
26+
27+
namespace paimon::test {
28+
29+
class TestOutputStream : public OutputStream {
30+
public:
31+
Result<int64_t> GetPos() const override {
32+
return 0;
33+
}
34+
35+
Result<int64_t> Write(const char*, int64_t size) override {
36+
return size;
37+
}
38+
39+
Status Flush() override {
40+
return Status::OK();
41+
}
42+
43+
Status Close() override {
44+
return Status::OK();
45+
}
46+
47+
Result<std::string> GetUri() const override {
48+
return std::string();
49+
}
50+
};
51+
52+
class TestJindoFileSystem : public jindo::JindoFileSystem {
53+
public:
54+
TestJindoFileSystem() : JindoFileSystem(std::make_unique<JdoFileSystem>()) {}
55+
56+
Result<bool> Exists(const std::string&) const override {
57+
return false;
58+
}
59+
60+
Status Mkdirs(const std::string& path) const override {
61+
parent_path_ = path;
62+
calls_.push_back("mkdirs");
63+
return mkdirs_status_;
64+
}
65+
66+
void SetMkdirsStatus(Status status) {
67+
mkdirs_status_ = std::move(status);
68+
}
69+
70+
const std::string& GetParentPath() const {
71+
return parent_path_;
72+
}
73+
74+
bool IsWriterOpened() const {
75+
return writer_opened_;
76+
}
77+
78+
const std::vector<std::string>& GetCalls() const {
79+
return calls_;
80+
}
81+
82+
protected:
83+
Result<std::unique_ptr<OutputStream>> OpenWriter(const std::string&) const override {
84+
calls_.push_back("open_writer");
85+
writer_opened_ = true;
86+
std::unique_ptr<OutputStream> output = std::make_unique<TestOutputStream>();
87+
return output;
88+
}
89+
90+
private:
91+
mutable std::string parent_path_;
92+
mutable bool writer_opened_ = false;
93+
mutable std::vector<std::string> calls_;
94+
Status mkdirs_status_ = Status::OK();
95+
};
96+
97+
TEST(JindoFileSystemUnitTest, CreateMakesParentDirectoryBeforeOpeningWriter) {
98+
TestJindoFileSystem fs;
99+
const std::string path = "oss://bucket/table/bucket-24/data.orc";
100+
101+
ASSERT_OK_AND_ASSIGN(std::unique_ptr<OutputStream> output,
102+
fs.Create(path, /*overwrite=*/false));
103+
ASSERT_TRUE(output);
104+
ASSERT_EQ(fs.GetParentPath(), "oss://bucket/table/bucket-24");
105+
ASSERT_TRUE(fs.IsWriterOpened());
106+
ASSERT_EQ(fs.GetCalls().size(), 2);
107+
ASSERT_EQ(fs.GetCalls()[0], "mkdirs");
108+
ASSERT_EQ(fs.GetCalls()[1], "open_writer");
109+
}
110+
111+
TEST(JindoFileSystemUnitTest, CreateSkipsObjectStoreRootParent) {
112+
TestJindoFileSystem fs;
113+
const std::string path = "oss://bucket/data.orc";
114+
115+
ASSERT_OK_AND_ASSIGN(std::unique_ptr<OutputStream> output,
116+
fs.Create(path, /*overwrite=*/false));
117+
ASSERT_TRUE(output);
118+
ASSERT_TRUE(fs.GetParentPath().empty());
119+
ASSERT_TRUE(fs.IsWriterOpened());
120+
ASSERT_EQ(fs.GetCalls().size(), 1);
121+
ASSERT_EQ(fs.GetCalls()[0], "open_writer");
122+
}
123+
124+
TEST(JindoFileSystemUnitTest, CreateReturnsParentDirectoryFailure) {
125+
TestJindoFileSystem fs;
126+
const std::string path = "oss://bucket/table/bucket-24/data.orc";
127+
fs.SetMkdirsStatus(Status::IOError("failed to create parent directory"));
128+
129+
ASSERT_NOK_WITH_MSG(fs.Create(path, /*overwrite=*/false), "failed to create parent directory");
130+
ASSERT_EQ(fs.GetParentPath(), "oss://bucket/table/bucket-24");
131+
ASSERT_FALSE(fs.IsWriterOpened());
132+
ASSERT_EQ(fs.GetCalls().size(), 1);
133+
ASSERT_EQ(fs.GetCalls()[0], "mkdirs");
134+
}
135+
136+
} // namespace paimon::test

0 commit comments

Comments
 (0)