This is an automated email from the ASF dual-hosted git repository.
gangwu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new b8f5c49f feat: Add error collection pattern to PendingUpdate (#358)
b8f5c49f is described below
commit b8f5c49f5dd62276e902f49c01efda1336f35da1
Author: Xinli Shang <[email protected]>
AuthorDate: Mon Dec 1 17:51:34 2025 -0800
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]>
---
src/iceberg/pending_update.h | 3 +-
src/iceberg/table_metadata.cc | 16 +----
src/iceberg/table_metadata.h | 5 +-
src/iceberg/util/error_collector.h | 139 +++++++++++++++++++++++++++++++++++++
src/iceberg/util/meson.build | 1 +
5 files changed, 148 insertions(+), 16 deletions(-)
diff --git a/src/iceberg/pending_update.h b/src/iceberg/pending_update.h
index 4c6fe095..8c1f7f27 100644
--- a/src/iceberg/pending_update.h
+++ b/src/iceberg/pending_update.h
@@ -25,6 +25,7 @@
#include "iceberg/iceberg_export.h"
#include "iceberg/result.h"
#include "iceberg/type_fwd.h"
+#include "iceberg/util/error_collector.h"
namespace iceberg {
@@ -74,7 +75,7 @@ class ICEBERG_EXPORT PendingUpdate {
///
/// \tparam T The type of result returned by Apply()
template <typename T>
-class ICEBERG_EXPORT PendingUpdateTyped : public PendingUpdate {
+class ICEBERG_EXPORT PendingUpdateTyped : public PendingUpdate, public
ErrorCollector {
public:
~PendingUpdateTyped() override = default;
diff --git a/src/iceberg/table_metadata.cc b/src/iceberg/table_metadata.cc
index df23e2f9..393eebfc 100644
--- a/src/iceberg/table_metadata.cc
+++ b/src/iceberg/table_metadata.cc
@@ -275,9 +275,6 @@ struct TableMetadataBuilder::Impl {
// Change tracking
std::vector<std::unique_ptr<TableUpdate>> changes;
- // Error collection (since methods return *this and cannot throw)
- std::vector<Error> errors;
-
// Metadata location tracking
std::optional<std::string> metadata_location;
std::optional<std::string> previous_metadata_location;
@@ -348,8 +345,7 @@ TableMetadataBuilder&
TableMetadataBuilder::AssignUUID(std::string_view uuid) {
// Validation: UUID cannot be empty
if (uuid_str.empty()) {
- impl_->errors.emplace_back(ErrorKind::kInvalidArgument, "Cannot assign
empty UUID");
- return *this;
+ return AddError(ErrorKind::kInvalidArgument, "Cannot assign empty UUID");
}
// Check if UUID is already set to the same value (no-op)
@@ -451,7 +447,7 @@ TableMetadataBuilder& TableMetadataBuilder::RemoveSnapshots(
throw IcebergError(std::format("{} not implemented", __FUNCTION__));
}
-TableMetadataBuilder& TableMetadataBuilder::suppressHistoricalSnapshots() {
+TableMetadataBuilder& TableMetadataBuilder::SuppressHistoricalSnapshots() {
throw IcebergError(std::format("{} not implemented", __FUNCTION__));
}
@@ -499,13 +495,7 @@ TableMetadataBuilder&
TableMetadataBuilder::RemoveEncryptionKey(std::string_view
Result<std::unique_ptr<TableMetadata>> TableMetadataBuilder::Build() {
// 1. Check for accumulated errors
- if (!impl_->errors.empty()) {
- std::string error_msg = "Failed to build TableMetadata due to validation
errors:\n";
- for (const auto& [kind, message] : impl_->errors) {
- error_msg += " - " + message + "\n";
- }
- return CommitFailed("{}", error_msg);
- }
+ ICEBERG_RETURN_UNEXPECTED(CheckErrors());
// 2. Validate metadata consistency through TableMetadata#Validate
diff --git a/src/iceberg/table_metadata.h b/src/iceberg/table_metadata.h
index 5ecbe476..503b9b14 100644
--- a/src/iceberg/table_metadata.h
+++ b/src/iceberg/table_metadata.h
@@ -30,6 +30,7 @@
#include "iceberg/iceberg_export.h"
#include "iceberg/type_fwd.h"
+#include "iceberg/util/error_collector.h"
#include "iceberg/util/lazy.h"
#include "iceberg/util/timepoint.h"
@@ -193,7 +194,7 @@ ICEBERG_EXPORT std::string ToString(const MetadataLogEntry&
entry);
/// If a modification violates Iceberg table constraints (e.g., setting a
current
/// schema ID that does not exist), an error will be recorded and returned when
/// Build() is called.
-class ICEBERG_EXPORT TableMetadataBuilder {
+class ICEBERG_EXPORT TableMetadataBuilder : public ErrorCollector {
public:
/// \brief Create a builder for a new table
///
@@ -353,7 +354,7 @@ class ICEBERG_EXPORT TableMetadataBuilder {
/// RemoveSnapshot changes are created. A snapshot is historical if no ref
directly
/// references its ID.
/// \return Reference to this builder for method chaining
- TableMetadataBuilder& suppressHistoricalSnapshots();
+ TableMetadataBuilder& SuppressHistoricalSnapshots();
/// \brief Set table statistics
///
diff --git a/src/iceberg/util/error_collector.h
b/src/iceberg/util/error_collector.h
new file mode 100644
index 00000000..f94967f9
--- /dev/null
+++ b/src/iceberg/util/error_collector.h
@@ -0,0 +1,139 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+/// \file iceberg/util/error_collector.h
+/// Base class for collecting validation errors in builder patterns
+
+#include <string>
+#include <vector>
+
+#include "iceberg/iceberg_export.h"
+#include "iceberg/result.h"
+
+namespace iceberg {
+
+/// \brief Base class for collecting validation errors in builder patterns
+///
+/// This class provides error accumulation functionality for builders that
+/// cannot throw exceptions. Builder methods can call AddError() to accumulate
+/// validation errors, and CheckErrors() returns all errors at once.
+///
+/// Example usage:
+/// \code
+/// class MyBuilder : public ErrorCollectorBase {
+/// public:
+/// MyBuilder& SetValue(int val) {
+/// if (val < 0) {
+/// return AddError(ErrorKind::kInvalidArgument, "Value must be
non-negative");
+/// }
+/// value_ = val;
+/// return *this;
+/// }
+///
+/// Result<MyObject> Build() {
+/// ICEBERG_RETURN_UNEXPECTED(CheckErrors());
+/// return MyObject{value_};
+/// }
+///
+/// private:
+/// int value_ = 0;
+/// };
+/// \endcode
+class ICEBERG_EXPORT ErrorCollector {
+ public:
+ ErrorCollector() = default;
+ virtual ~ErrorCollector() = default;
+
+ ErrorCollector(ErrorCollector&&) = default;
+ ErrorCollector& operator=(ErrorCollector&&) = default;
+
+ ErrorCollector(const ErrorCollector&) = default;
+ ErrorCollector& operator=(const ErrorCollector&) = default;
+
+ /// \brief Add a specific error and return reference to derived class
+ ///
+ /// \param self Deduced reference to the derived class instance
+ /// \param kind The kind of error
+ /// \param message The error message
+ /// \return Reference to the derived class for method chaining
+ auto& AddError(this auto& self, ErrorKind kind, std::string message) {
+ self.errors_.emplace_back(kind, std::move(message));
+ return self;
+ }
+
+ /// \brief Add an existing error object and return reference to derived class
+ ///
+ /// Useful when propagating errors from other components or reusing
+ /// error objects without deconstructing and reconstructing them.
+ ///
+ /// \param self Deduced reference to the derived class instance
+ /// \param err The error to add
+ /// \return Reference to the derived class for method chaining
+ auto& AddError(this auto& self, Error err) {
+ self.errors_.push_back(std::move(err));
+ return self;
+ }
+
+ /// \brief Check if any errors have been collected
+ ///
+ /// \return true if there are accumulated errors
+ [[nodiscard]] bool HasErrors() const { return !errors_.empty(); }
+
+ /// \brief Get the number of errors collected
+ ///
+ /// \return The count of accumulated errors
+ [[nodiscard]] size_t ErrorCount() const { return errors_.size(); }
+
+ /// \brief Check for accumulated errors and return them if any exist
+ ///
+ /// This should be called before completing a builder operation (e.g.,
+ /// in Build(), Apply(), or Commit() methods) to validate that no errors
+ /// were accumulated during the builder method calls.
+ ///
+ /// \return Status::OK if no errors, or a ValidationFailed error with
+ /// all accumulated error messages
+ [[nodiscard]] Status CheckErrors() const {
+ if (!errors_.empty()) {
+ std::string error_msg = "Validation failed due to the following
errors:\n";
+ for (const auto& [kind, message] : errors_) {
+ error_msg += " - " + message + "\n";
+ }
+ return ValidationFailed("{}", error_msg);
+ }
+ return {};
+ }
+
+ /// \brief Clear all accumulated errors
+ ///
+ /// This can be useful for resetting the error state in tests or
+ /// when reusing a builder instance.
+ void ClearErrors() { errors_.clear(); }
+
+ /// \brief Get read-only access to all collected errors
+ ///
+ /// \return A const reference to the vector of errors
+ [[nodiscard]] const std::vector<Error>& Errors() const { return errors_; }
+
+ protected:
+ std::vector<Error> errors_;
+};
+
+} // namespace iceberg
diff --git a/src/iceberg/util/meson.build b/src/iceberg/util/meson.build
index 30c42d02..0e050847 100644
--- a/src/iceberg/util/meson.build
+++ b/src/iceberg/util/meson.build
@@ -23,6 +23,7 @@ install_headers(
'conversions.h',
'decimal.h',
'endian.h',
+ 'error_collector.h',
'formattable.h',
'formatter.h',
'int128.h',