zeroshade commented on code in PR #431:
URL: https://github.com/apache/iceberg-go/pull/431#discussion_r2132919008


##########
table/update_schema.go:
##########
@@ -0,0 +1,532 @@
+package table
+
+import (
+       "github.com/apache/iceberg-go"
+)
+
+type UpdateSchema struct {
+       base                     *Metadata
+       schema                   *iceberg.Schema
+       idToParent               map[int]int
+       deletes                  []int
+       updates                  map[int]*iceberg.NestedField
+       parentToAddedIDs         map[int][]int
+       addedNameToID            map[string]int
+       lastColumnID             int
+       allowIncompatibleChanges bool
+       identifierFields         map[string]struct{}
+       caseSensitive            bool
+}
+
+func NewUpdateSchema(base *Metadata, s *iceberg.Schema, lastColumnID int) 
*UpdateSchema {
+       identifierFields := make(map[string]struct{})
+
+       return &UpdateSchema{
+               base:                     base,
+               schema:                   s,
+               idToParent:               make(map[int]int),
+               deletes:                  make([]int, 0),
+               updates:                  make(map[int]*iceberg.NestedField),
+               parentToAddedIDs:         make(map[int][]int),
+               addedNameToID:            make(map[string]int),
+               lastColumnID:             lastColumnID,
+               allowIncompatibleChanges: false,
+               identifierFields:         identifierFields,
+               caseSensitive:            true,
+       }
+}
+
+// AllowIncompatibleChanges permits incompatible schema changes.
+func (us *UpdateSchema) AllowIncompatibleChanges() *UpdateSchema {
+       us.allowIncompatibleChanges = true
+
+       return us
+}
+
+func (us *UpdateSchema) AddColumn(parent, name string, new_id int, required 
bool, dataType iceberg.Type, doc string, initialDefaultValue any) *UpdateSchema 
{
+       parentID := -1
+       fullName := ""
+
+       if parent != "" {
+               parentField := us.findField(parent)
+               if parentField == nil {
+                       panic("Cannot find parent struct: " + parent)
+               }
+
+               // Get the parent struct's ID
+               parentID = parentField.ID
+
+               // Store the parent-child relationship
+               us.idToParent[new_id] = parentID // Add this line
+               if _, ok := us.parentToAddedIDs[parentID]; !ok {
+                       us.parentToAddedIDs[parentID] = make([]int, 0)
+               }
+               us.parentToAddedIDs[parentID] = 
append(us.parentToAddedIDs[parentID], new_id)

Review Comment:
   this logic can be simplified:
   
   if there's a parent, we determine the parent field, parent ID and ensure 
that the parent is a Map, List or Struct type. After that, all of the logic 
should be the same regardless of whether there's a parent or not (no parent 
would mean the parent ID is -1 which indicates it's a top level field, etc.)
   
   That way we can avoid the duplication of the logic.



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to