zeroshade commented on code in PR #1415:
URL: https://github.com/apache/iceberg-go/pull/1415#discussion_r3553634151
##########
schema.go:
##########
@@ -257,7 +257,7 @@ func (s *Schema) Type() string { return "struct" }
// AsStruct returns a Struct with the same fields as the schema which can
// then be used as a Type.
-func (s *Schema) AsStruct() StructType { return StructType{FieldList:
s.fields} }
+func (s *Schema) AsStruct() StructType { return StructType{FieldList:
cloneFields(s.fields)} }
func (s *Schema) NumFields() int { return len(s.fields) }
Review Comment:
`AsStruct()` is on the read-only scan critical path (called by schema
visitors and evaluator constructors that never mutate the result). Deep-cloning
the full field tree on every call is a real regression for wide/nested schemas.
Suggest a shared/private `asStructRef()` for internal read-only callers,
keeping the deep clone only in the exported `AsStruct()`.
##########
schema.go:
##########
@@ -310,6 +310,44 @@ func (s *Schema) MarshalJSON() ([]byte, error) {
}{Type: "struct", Fields: s.fields, Alias: &aliasCopy})
}
+func cloneFields(fields []NestedField) []NestedField {
+ if len(fields) == 0 {
+ return nil
+ }
+
+ cloned := make([]NestedField, len(fields))
+ for i, field := range fields {
+ cloned[i] = field
+ cloned[i].Type = cloneType(field.Type)
+ }
+
+ return cloned
+}
+
+func cloneType(t Type) Type {
+ if t == nil {
+ return nil
+ }
+
+ switch typed := t.(type) {
+ case *StructType:
+ return &StructType{FieldList: cloneFields(typed.FieldList)}
+ case *ListType:
+ cloned := *typed
+ cloned.Element = cloneType(typed.Element)
+
+ return &cloned
+ case *MapType:
+ cloned := *typed
+ cloned.KeyType = cloneType(typed.KeyType)
+ cloned.ValueType = cloneType(typed.ValueType)
+
+ return &cloned
+ default:
Review Comment:
The `default` branch returns `t` unchanged
(primitive/Decimal/Fixed/Geometry/Geography/Variant) — likely intentional since
these are immutable values, but a one-line comment would keep it from reading
as a missed case.
--
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]
