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',

Reply via email to