Skip to content

Commit 04cef13

Browse files
committed
feat(io): add bulk delete API to FileIO.
1 parent 2b4e2d4 commit 04cef13

3 files changed

Lines changed: 16 additions & 22 deletions

File tree

src/iceberg/file_io.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ class ICEBERG_EXPORT FileIO {
151151
///
152152
/// \param file_location The location of the file to delete.
153153
/// \return void if the delete succeeded, an error code if the delete failed.
154-
virtual Status DeleteFile(const std::string&) {
154+
virtual Status DeleteFile(const std::string& file_location) {
155155
return NotImplemented("DeleteFile not implemented");
156156
}
157157

src/iceberg/test/file_io_test.cc

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,17 @@
2727

2828
#include "iceberg/test/matchers.h"
2929

30-
namespace iceberg {
3130
namespace {
3231

33-
class RecordingFileIO : public FileIO {
32+
class RecordingFileIO : public iceberg::FileIO {
3433
public:
3534
explicit RecordingFileIO(std::string failure_path = "")
3635
: failure_path_(std::move(failure_path)) {}
3736

38-
Status DeleteFile(const std::string& file_location) override {
37+
iceberg::Status DeleteFile(const std::string& file_location) override {
3938
deleted_paths.push_back(file_location);
4039
if (file_location == failure_path_) {
41-
return IOError("failed to delete {}", file_location);
40+
return iceberg::IOError("failed to delete {}", file_location);
4241
}
4342
return {};
4443
}
@@ -49,11 +48,13 @@ class RecordingFileIO : public FileIO {
4948
std::string failure_path_;
5049
};
5150

51+
} // namespace
52+
5253
TEST(FileIOTest, DeleteFilesFallsBackToDeleteFileForEachPath) {
5354
RecordingFileIO file_io;
5455
std::vector<std::string> paths = {"file-a.avro", "file-b.avro"};
5556

56-
EXPECT_THAT(file_io.DeleteFiles(paths), IsOk());
57+
EXPECT_THAT(file_io.DeleteFiles(paths), iceberg::IsOk());
5758
EXPECT_THAT(file_io.deleted_paths,
5859
::testing::ElementsAre("file-a.avro", "file-b.avro"));
5960
}
@@ -64,11 +65,8 @@ TEST(FileIOTest, DeleteFilesReturnsFirstDeleteFileError) {
6465

6566
auto status = file_io.DeleteFiles(paths);
6667

67-
EXPECT_THAT(status, IsError(ErrorKind::kIOError));
68-
EXPECT_THAT(status, HasErrorMessage("failed to delete file-b.avro"));
68+
EXPECT_THAT(status, iceberg::IsError(iceberg::ErrorKind::kIOError));
69+
EXPECT_THAT(status, iceberg::HasErrorMessage("failed to delete file-b.avro"));
6970
EXPECT_THAT(file_io.deleted_paths,
7071
::testing::ElementsAre("file-a.avro", "file-b.avro"));
7172
}
72-
73-
} // namespace
74-
} // namespace iceberg

src/iceberg/update/expire_snapshots.cc

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -75,26 +75,22 @@ class FileCleanupStrategy {
7575
CleanupLevel level) = 0;
7676

7777
protected:
78-
/// \brief Delete a single file
79-
void DeleteFile(const std::string& path) {
78+
/// \brief Delete files at the given locations.
79+
void DeleteFiles(const std::unordered_set<std::string>& paths) {
8080
try {
8181
if (delete_func_) {
82-
delete_func_(path);
82+
for (const auto& path : paths) {
83+
delete_func_(path);
84+
}
8385
} else {
84-
std::ignore = file_io_->DeleteFile(path);
86+
std::vector<std::string> path_list(paths.begin(), paths.end());
87+
std::ignore = file_io_->DeleteFiles(path_list);
8588
}
8689
} catch (...) {
8790
/// TODO(shangxinli): add retry
8891
}
8992
}
9093

91-
/// TODO(shangxinli): Add bulk deletion
92-
void DeleteFiles(const std::unordered_set<std::string>& paths) {
93-
for (const auto& path : paths) {
94-
DeleteFile(path);
95-
}
96-
}
97-
9894
bool HasAnyStatisticsFiles(const TableMetadata& metadata) const {
9995
return !metadata.statistics.empty() || !metadata.partition_statistics.empty();
10096
}

0 commit comments

Comments
 (0)