wgtmac commented on code in PR #486:
URL: https://github.com/apache/iceberg-cpp/pull/486#discussion_r2663981229


##########
src/iceberg/update/update_schema.cc:
##########
@@ -19,23 +19,212 @@
 
 #include "iceberg/update/update_schema.h"
 
+#include <format>
 #include <memory>
 #include <optional>
 #include <ranges>
 #include <string>
 #include <string_view>
 #include <unordered_set>
 #include <utility>
+#include <vector>
 
 #include "iceberg/schema.h"
+#include "iceberg/schema_field.h"
 #include "iceberg/table_metadata.h"
 #include "iceberg/transaction.h"
 #include "iceberg/type.h"
+#include "iceberg/util/checked_cast.h"
 #include "iceberg/util/error_collector.h"
 #include "iceberg/util/macros.h"
+#include "iceberg/util/type_util.h"
+#include "iceberg/util/visit_type.h"
 
 namespace iceberg {
 
+namespace {
+constexpr int32_t kTableRootId = -1;
+
+/// \brief Visitor for applying schema changes recursively to nested types
+class ApplyChangesVisitor {
+ public:
+  ApplyChangesVisitor(
+      const std::unordered_set<int32_t>& deletes,
+      const std::unordered_map<int32_t, std::shared_ptr<SchemaField>>& updates,
+      const std::unordered_map<int32_t, std::vector<int32_t>>& 
parent_to_added_ids)
+      : deletes_(deletes), updates_(updates), 
parent_to_added_ids_(parent_to_added_ids) {}
+
+  /// \brief Apply changes to a type using schema visitor pattern
+  Result<std::shared_ptr<Type>> ApplyChanges(const std::shared_ptr<Type>& type,
+                                             int32_t parent_id) {
+    return VisitSchemaInline(*type, this, type, parent_id);
+  }
+
+  /// \brief Apply changes to a struct type
+  Result<std::shared_ptr<Type>> VisitStruct(const StructType& struct_type,
+                                            const std::shared_ptr<Type>& 
base_type,
+                                            int32_t parent_id) {
+    std::vector<SchemaField> new_fields;
+
+    // Process existing fields
+    for (const auto& field : struct_type.fields()) {
+      // Recursively process the field's type first
+      ICEBERG_ASSIGN_OR_RAISE(auto field_type_result,
+                              ApplyChanges(field.type(), field.field_id()));
+
+      // Process field-level changes (deletes, updates, nested additions)
+      ICEBERG_ASSIGN_OR_RAISE(auto processed_field,
+                              ProcessField(field, field_type_result));
+
+      if (processed_field.has_value()) {
+        new_fields.push_back(std::move(processed_field.value()));
+      }
+    }
+
+    // Add new fields for this struct
+    auto adds_it = parent_to_added_ids_.find(parent_id);
+    if (adds_it != parent_to_added_ids_.end()) {
+      for (int32_t added_id : adds_it->second) {
+        auto added_field_it = updates_.find(added_id);
+        if (added_field_it != updates_.end()) {
+          new_fields.push_back(*added_field_it->second);
+        }
+      }
+    }
+
+    return std::make_shared<StructType>(std::move(new_fields));

Review Comment:
   Do we want to apply similar optimization to return original struct type if 
nothing changes?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to