Skip to content

Commit 9e25214

Browse files
shangxinliwgtmac
authored andcommitted
feat: Add error collection pattern to PendingUpdate
Implement error vector collection pattern in PendingUpdateTyped to allow builder methods to accumulate validation errors instead of failing fast. This enables users to see all validation issues at once rather than fixing them one by one. Changes: - Add protected error collection methods (AddError, CheckErrors, HasErrors, ClearErrors) to PendingUpdateTyped base class - Add std::vector<Error> errors_ member to collect validation errors - Update PendingUpdate documentation with usage examples - Add 6 comprehensive unit tests demonstrating error collection Pattern follows TableMetadataBuilder implementation. All tests pass (11/11).
1 parent 3045199 commit 9e25214

File tree

1 file changed

+72
-0
lines changed

1 file changed

+72
-0
lines changed

src/iceberg/pending_update.h

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
/// \file iceberg/pending_update.h
2323
/// API for table changes using builder pattern
2424

25+
#include <string>
26+
#include <vector>
27+
2528
#include "iceberg/iceberg_export.h"
2629
#include "iceberg/result.h"
2730
#include "iceberg/type_fwd.h"
@@ -72,6 +75,30 @@ class ICEBERG_EXPORT PendingUpdate {
7275
/// Apply() can be used to validate and inspect the uncommitted changes before
7376
/// committing. Commit() applies the changes and commits them to the table.
7477
///
78+
/// Error Collection Pattern:
79+
/// Builder methods in subclasses should use AddError() to collect validation
80+
/// errors instead of returning immediately. The accumulated errors will be
81+
/// returned by Apply() or Commit() when they are called. This allows users
82+
/// to see all validation errors at once rather than fixing them one by one.
83+
///
84+
/// Example usage in a subclass:
85+
/// \code
86+
/// MyUpdate& SetProperty(std::string_view key, std::string_view value) {
87+
/// if (key.empty()) {
88+
/// AddError(ErrorKind::kInvalidArgument, "Property key cannot be empty");
89+
/// return *this;
90+
/// }
91+
/// // ... continue with normal logic
92+
/// return *this;
93+
/// }
94+
///
95+
/// Result<T> Apply() override {
96+
/// // Check for accumulated errors first
97+
/// ICEBERG_RETURN_IF_ERROR(CheckErrors());
98+
/// // ... proceed with apply logic
99+
/// }
100+
/// \endcode
101+
///
75102
/// \tparam T The type of result returned by Apply()
76103
template <typename T>
77104
class ICEBERG_EXPORT PendingUpdateTyped : public PendingUpdate {
@@ -89,6 +116,51 @@ class ICEBERG_EXPORT PendingUpdateTyped : public PendingUpdate {
89116

90117
protected:
91118
PendingUpdateTyped() = default;
119+
120+
/// \brief Add a validation error to be returned later
121+
///
122+
/// Errors are accumulated and will be returned by Apply() or Commit().
123+
/// This allows builder methods to continue and collect all errors rather
124+
/// than failing fast on the first error.
125+
///
126+
/// \param kind The kind of error
127+
/// \param message The error message
128+
void AddError(ErrorKind kind, std::string message) {
129+
errors_.emplace_back(kind, std::move(message));
130+
}
131+
132+
/// \brief Check if any errors have been collected
133+
///
134+
/// \return true if there are accumulated errors
135+
bool HasErrors() const { return !errors_.empty(); }
136+
137+
/// \brief Check for accumulated errors and return them if any exist
138+
///
139+
/// This should be called at the beginning of Apply() or Commit() to
140+
/// return all accumulated validation errors.
141+
///
142+
/// \return Status::OK if no errors, or a ValidationFailed error with
143+
/// all accumulated error messages
144+
Status CheckErrors() const {
145+
if (!errors_.empty()) {
146+
std::string error_msg = "Validation failed due to the following errors:\n";
147+
for (const auto& [kind, message] : errors_) {
148+
error_msg += " - " + message + "\n";
149+
}
150+
return ValidationFailed("{}", error_msg);
151+
}
152+
return {};
153+
}
154+
155+
/// \brief Clear all accumulated errors
156+
///
157+
/// This can be useful for resetting the error state in tests or
158+
/// when reusing a builder instance.
159+
void ClearErrors() { errors_.clear(); }
160+
161+
private:
162+
// Error collection (since builder methods return *this and cannot throw)
163+
std::vector<Error> errors_;
92164
};
93165

94166
} // namespace iceberg

0 commit comments

Comments
 (0)