Skip to content

Commit b8f5c49

Browse files
shangxinliwgtmac
andauthored
feat: Add error collection pattern to PendingUpdate (#358)
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 Pattern follows TableMetadataBuilder implementation. --------- Co-authored-by: Gang Wu <[email protected]>
1 parent 3045199 commit b8f5c49

File tree

5 files changed

+148
-16
lines changed

5 files changed

+148
-16
lines changed

src/iceberg/pending_update.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "iceberg/iceberg_export.h"
2626
#include "iceberg/result.h"
2727
#include "iceberg/type_fwd.h"
28+
#include "iceberg/util/error_collector.h"
2829

2930
namespace iceberg {
3031

@@ -74,7 +75,7 @@ class ICEBERG_EXPORT PendingUpdate {
7475
///
7576
/// \tparam T The type of result returned by Apply()
7677
template <typename T>
77-
class ICEBERG_EXPORT PendingUpdateTyped : public PendingUpdate {
78+
class ICEBERG_EXPORT PendingUpdateTyped : public PendingUpdate, public ErrorCollector {
7879
public:
7980
~PendingUpdateTyped() override = default;
8081

src/iceberg/table_metadata.cc

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,6 @@ struct TableMetadataBuilder::Impl {
275275
// Change tracking
276276
std::vector<std::unique_ptr<TableUpdate>> changes;
277277

278-
// Error collection (since methods return *this and cannot throw)
279-
std::vector<Error> errors;
280-
281278
// Metadata location tracking
282279
std::optional<std::string> metadata_location;
283280
std::optional<std::string> previous_metadata_location;
@@ -348,8 +345,7 @@ TableMetadataBuilder& TableMetadataBuilder::AssignUUID(std::string_view uuid) {
348345

349346
// Validation: UUID cannot be empty
350347
if (uuid_str.empty()) {
351-
impl_->errors.emplace_back(ErrorKind::kInvalidArgument, "Cannot assign empty UUID");
352-
return *this;
348+
return AddError(ErrorKind::kInvalidArgument, "Cannot assign empty UUID");
353349
}
354350

355351
// Check if UUID is already set to the same value (no-op)
@@ -451,7 +447,7 @@ TableMetadataBuilder& TableMetadataBuilder::RemoveSnapshots(
451447
throw IcebergError(std::format("{} not implemented", __FUNCTION__));
452448
}
453449

454-
TableMetadataBuilder& TableMetadataBuilder::suppressHistoricalSnapshots() {
450+
TableMetadataBuilder& TableMetadataBuilder::SuppressHistoricalSnapshots() {
455451
throw IcebergError(std::format("{} not implemented", __FUNCTION__));
456452
}
457453

@@ -499,13 +495,7 @@ TableMetadataBuilder& TableMetadataBuilder::RemoveEncryptionKey(std::string_view
499495

500496
Result<std::unique_ptr<TableMetadata>> TableMetadataBuilder::Build() {
501497
// 1. Check for accumulated errors
502-
if (!impl_->errors.empty()) {
503-
std::string error_msg = "Failed to build TableMetadata due to validation errors:\n";
504-
for (const auto& [kind, message] : impl_->errors) {
505-
error_msg += " - " + message + "\n";
506-
}
507-
return CommitFailed("{}", error_msg);
508-
}
498+
ICEBERG_RETURN_UNEXPECTED(CheckErrors());
509499

510500
// 2. Validate metadata consistency through TableMetadata#Validate
511501

src/iceberg/table_metadata.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
#include "iceberg/iceberg_export.h"
3232
#include "iceberg/type_fwd.h"
33+
#include "iceberg/util/error_collector.h"
3334
#include "iceberg/util/lazy.h"
3435
#include "iceberg/util/timepoint.h"
3536

@@ -193,7 +194,7 @@ ICEBERG_EXPORT std::string ToString(const MetadataLogEntry& entry);
193194
/// If a modification violates Iceberg table constraints (e.g., setting a current
194195
/// schema ID that does not exist), an error will be recorded and returned when
195196
/// Build() is called.
196-
class ICEBERG_EXPORT TableMetadataBuilder {
197+
class ICEBERG_EXPORT TableMetadataBuilder : public ErrorCollector {
197198
public:
198199
/// \brief Create a builder for a new table
199200
///
@@ -353,7 +354,7 @@ class ICEBERG_EXPORT TableMetadataBuilder {
353354
/// RemoveSnapshot changes are created. A snapshot is historical if no ref directly
354355
/// references its ID.
355356
/// \return Reference to this builder for method chaining
356-
TableMetadataBuilder& suppressHistoricalSnapshots();
357+
TableMetadataBuilder& SuppressHistoricalSnapshots();
357358

358359
/// \brief Set table statistics
359360
///

src/iceberg/util/error_collector.h

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
#pragma once
21+
22+
/// \file iceberg/util/error_collector.h
23+
/// Base class for collecting validation errors in builder patterns
24+
25+
#include <string>
26+
#include <vector>
27+
28+
#include "iceberg/iceberg_export.h"
29+
#include "iceberg/result.h"
30+
31+
namespace iceberg {
32+
33+
/// \brief Base class for collecting validation errors in builder patterns
34+
///
35+
/// This class provides error accumulation functionality for builders that
36+
/// cannot throw exceptions. Builder methods can call AddError() to accumulate
37+
/// validation errors, and CheckErrors() returns all errors at once.
38+
///
39+
/// Example usage:
40+
/// \code
41+
/// class MyBuilder : public ErrorCollectorBase {
42+
/// public:
43+
/// MyBuilder& SetValue(int val) {
44+
/// if (val < 0) {
45+
/// return AddError(ErrorKind::kInvalidArgument, "Value must be non-negative");
46+
/// }
47+
/// value_ = val;
48+
/// return *this;
49+
/// }
50+
///
51+
/// Result<MyObject> Build() {
52+
/// ICEBERG_RETURN_UNEXPECTED(CheckErrors());
53+
/// return MyObject{value_};
54+
/// }
55+
///
56+
/// private:
57+
/// int value_ = 0;
58+
/// };
59+
/// \endcode
60+
class ICEBERG_EXPORT ErrorCollector {
61+
public:
62+
ErrorCollector() = default;
63+
virtual ~ErrorCollector() = default;
64+
65+
ErrorCollector(ErrorCollector&&) = default;
66+
ErrorCollector& operator=(ErrorCollector&&) = default;
67+
68+
ErrorCollector(const ErrorCollector&) = default;
69+
ErrorCollector& operator=(const ErrorCollector&) = default;
70+
71+
/// \brief Add a specific error and return reference to derived class
72+
///
73+
/// \param self Deduced reference to the derived class instance
74+
/// \param kind The kind of error
75+
/// \param message The error message
76+
/// \return Reference to the derived class for method chaining
77+
auto& AddError(this auto& self, ErrorKind kind, std::string message) {
78+
self.errors_.emplace_back(kind, std::move(message));
79+
return self;
80+
}
81+
82+
/// \brief Add an existing error object and return reference to derived class
83+
///
84+
/// Useful when propagating errors from other components or reusing
85+
/// error objects without deconstructing and reconstructing them.
86+
///
87+
/// \param self Deduced reference to the derived class instance
88+
/// \param err The error to add
89+
/// \return Reference to the derived class for method chaining
90+
auto& AddError(this auto& self, Error err) {
91+
self.errors_.push_back(std::move(err));
92+
return self;
93+
}
94+
95+
/// \brief Check if any errors have been collected
96+
///
97+
/// \return true if there are accumulated errors
98+
[[nodiscard]] bool HasErrors() const { return !errors_.empty(); }
99+
100+
/// \brief Get the number of errors collected
101+
///
102+
/// \return The count of accumulated errors
103+
[[nodiscard]] size_t ErrorCount() const { return errors_.size(); }
104+
105+
/// \brief Check for accumulated errors and return them if any exist
106+
///
107+
/// This should be called before completing a builder operation (e.g.,
108+
/// in Build(), Apply(), or Commit() methods) to validate that no errors
109+
/// were accumulated during the builder method calls.
110+
///
111+
/// \return Status::OK if no errors, or a ValidationFailed error with
112+
/// all accumulated error messages
113+
[[nodiscard]] Status CheckErrors() const {
114+
if (!errors_.empty()) {
115+
std::string error_msg = "Validation failed due to the following errors:\n";
116+
for (const auto& [kind, message] : errors_) {
117+
error_msg += " - " + message + "\n";
118+
}
119+
return ValidationFailed("{}", error_msg);
120+
}
121+
return {};
122+
}
123+
124+
/// \brief Clear all accumulated errors
125+
///
126+
/// This can be useful for resetting the error state in tests or
127+
/// when reusing a builder instance.
128+
void ClearErrors() { errors_.clear(); }
129+
130+
/// \brief Get read-only access to all collected errors
131+
///
132+
/// \return A const reference to the vector of errors
133+
[[nodiscard]] const std::vector<Error>& Errors() const { return errors_; }
134+
135+
protected:
136+
std::vector<Error> errors_;
137+
};
138+
139+
} // namespace iceberg

src/iceberg/util/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ install_headers(
2323
'conversions.h',
2424
'decimal.h',
2525
'endian.h',
26+
'error_collector.h',
2627
'formattable.h',
2728
'formatter.h',
2829
'int128.h',

0 commit comments

Comments
 (0)