Skip to content

Commit 4e37cbf

Browse files
committed
feat: add missing methods to SnapshotUpdate interface
Add DeleteWith() and ToBranch() methods to complete the SnapshotUpdate API coverage from Java implementation. This brings the interface closer to feature parity with Java Iceberg. New methods: - DeleteWith(): Set custom file deletion callback for tracking deleted files, implementing custom retention policies, or delegating deletion to external systems. Uses std::function<void(std::string_view)> for the callback. - ToBranch(): Commit snapshot to a specific branch instead of main branch. Useful for Write-Audit-Publish (WAP) workflows, feature branch development, and testing changes before merging to main. Method deferred: - ScanManifestsWith(): Deferred until executor/thread pool infrastructure is available in the codebase. Documented in class comments for future addition. Changes: - Add DeleteWith() and ToBranch() methods with fluent API - Add delete_func_ and target_branch_ protected members - Add 3 new test cases: DeleteWith, ToBranch, MethodChainingWithAllMethods - Update class documentation to list included and deferred methods - Total test count increased from 10 to 13, all passing
1 parent 7223810 commit 4e37cbf

File tree

2 files changed

+100
-0
lines changed

2 files changed

+100
-0
lines changed

src/iceberg/snapshot_update.h

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
/// \file iceberg/snapshot_update.h
2323
/// API for table updates that produce snapshots
2424

25+
#include <functional>
26+
#include <optional>
2527
#include <string>
2628
#include <string_view>
2729
#include <unordered_map>
@@ -42,6 +44,16 @@ namespace iceberg {
4244
/// fluent API method chaining in derived classes, matching the Java pattern
4345
/// where SnapshotUpdate<ThisT> allows methods to return the actual derived type.
4446
///
47+
/// Methods included from Java API:
48+
/// - Set(): Set summary properties
49+
/// - StageOnly(): Stage without updating current snapshot
50+
/// - DeleteWith(): Custom file deletion callback
51+
/// - ToBranch(): Commit to a specific branch
52+
///
53+
/// Methods deferred (will be added when infrastructure is available):
54+
/// - ScanManifestsWith(): Custom executor for parallel manifest scanning
55+
/// (requires executor/thread pool infrastructure)
56+
///
4557
/// \tparam Derived The actual implementation class (e.g., AppendFiles)
4658
template <typename Derived>
4759
class ICEBERG_EXPORT SnapshotUpdate : public PendingUpdateTyped<Snapshot> {
@@ -74,6 +86,36 @@ class ICEBERG_EXPORT SnapshotUpdate : public PendingUpdateTyped<Snapshot> {
7486
return static_cast<Derived&>(*this);
7587
}
7688

89+
/// \brief Set a custom file deletion callback
90+
///
91+
/// By default, files are deleted using the table's FileIO implementation.
92+
/// This method allows providing a custom deletion callback for use cases like:
93+
/// - Tracking deleted files for auditing
94+
/// - Implementing custom retention policies
95+
/// - Delegating deletion to external systems
96+
///
97+
/// \param delete_func Callback function that will be called for each file to delete
98+
/// \return Reference to derived class for method chaining
99+
Derived& DeleteWith(std::function<void(std::string_view)> delete_func) {
100+
delete_func_ = std::move(delete_func);
101+
return static_cast<Derived&>(*this);
102+
}
103+
104+
/// \brief Commit the snapshot to a specific branch
105+
///
106+
/// By default, snapshots are committed to the table's main branch.
107+
/// This method allows committing to a named branch instead, which is useful for:
108+
/// - Write-Audit-Publish (WAP) workflows
109+
/// - Feature branch development
110+
/// - Testing changes before merging to main
111+
///
112+
/// \param branch The name of the branch to commit to
113+
/// \return Reference to derived class for method chaining
114+
Derived& ToBranch(std::string_view branch) {
115+
target_branch_ = std::string(branch);
116+
return static_cast<Derived&>(*this);
117+
}
118+
77119
protected:
78120
SnapshotUpdate() = default;
79121

@@ -82,6 +124,12 @@ class ICEBERG_EXPORT SnapshotUpdate : public PendingUpdateTyped<Snapshot> {
82124

83125
/// \brief Whether to stage only without updating current snapshot
84126
bool stage_only_ = false;
127+
128+
/// \brief Custom file deletion callback
129+
std::optional<std::function<void(std::string_view)>> delete_func_;
130+
131+
/// \brief Target branch name for commit (nullopt means main branch)
132+
std::optional<std::string> target_branch_;
85133
};
86134

87135
} // namespace iceberg

src/iceberg/test/snapshot_update_test.cc

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ class MockSnapshotUpdate : public SnapshotUpdate<MockSnapshotUpdate> {
6868
return summary_;
6969
}
7070
bool GetStageOnly() const { return stage_only_; }
71+
const std::optional<std::function<void(std::string_view)>>& GetDeleteFunc() const {
72+
return delete_func_;
73+
}
74+
const std::optional<std::string>& GetTargetBranch() const { return target_branch_; }
7175

7276
private:
7377
bool should_fail_ = false;
@@ -100,6 +104,33 @@ TEST(SnapshotUpdateTest, StageOnly) {
100104
EXPECT_TRUE(update.GetStageOnly());
101105
}
102106

107+
TEST(SnapshotUpdateTest, DeleteWith) {
108+
MockSnapshotUpdate update;
109+
std::vector<std::string> deleted_files;
110+
111+
update.DeleteWith([&deleted_files](std::string_view path) {
112+
deleted_files.push_back(std::string(path));
113+
});
114+
115+
EXPECT_TRUE(update.GetDeleteFunc().has_value());
116+
// Test the callback works
117+
if (update.GetDeleteFunc()) {
118+
(*update.GetDeleteFunc())("s3://bucket/data/file1.parquet");
119+
(*update.GetDeleteFunc())("s3://bucket/data/file2.parquet");
120+
}
121+
EXPECT_EQ(deleted_files.size(), 2);
122+
EXPECT_EQ(deleted_files[0], "s3://bucket/data/file1.parquet");
123+
EXPECT_EQ(deleted_files[1], "s3://bucket/data/file2.parquet");
124+
}
125+
126+
TEST(SnapshotUpdateTest, ToBranch) {
127+
MockSnapshotUpdate update;
128+
update.ToBranch("feature-branch");
129+
130+
EXPECT_TRUE(update.GetTargetBranch().has_value());
131+
EXPECT_EQ(*update.GetTargetBranch(), "feature-branch");
132+
}
133+
103134
TEST(SnapshotUpdateTest, MethodChaining) {
104135
MockSnapshotUpdate update;
105136
update.Set("operation", "append")
@@ -114,6 +145,27 @@ TEST(SnapshotUpdateTest, MethodChaining) {
114145
EXPECT_TRUE(update.GetStageOnly());
115146
}
116147

148+
TEST(SnapshotUpdateTest, MethodChainingWithAllMethods) {
149+
MockSnapshotUpdate update;
150+
std::vector<std::string> deleted_files;
151+
152+
update.Set("operation", "append")
153+
.Set("added-files-count", "5")
154+
.DeleteWith([&deleted_files](std::string_view path) {
155+
deleted_files.push_back(std::string(path));
156+
})
157+
.ToBranch("wap-branch")
158+
.StageOnly();
159+
160+
EXPECT_EQ(update.GetSummary().size(), 2);
161+
EXPECT_EQ(update.GetSummary().at("operation"), "append");
162+
EXPECT_EQ(update.GetSummary().at("added-files-count"), "5");
163+
EXPECT_TRUE(update.GetDeleteFunc().has_value());
164+
EXPECT_TRUE(update.GetTargetBranch().has_value());
165+
EXPECT_EQ(*update.GetTargetBranch(), "wap-branch");
166+
EXPECT_TRUE(update.GetStageOnly());
167+
}
168+
117169
TEST(SnapshotUpdateTest, ApplySuccess) {
118170
MockSnapshotUpdate update;
119171
auto result = update.Apply();

0 commit comments

Comments
 (0)