Revanth14 commented on code in PR #2372:
URL: https://github.com/apache/iceberg-go/pull/2372#discussion_r4461057145
##########
defaults.go:
##########
@@ -49,3 +53,108 @@ func CloneDefaultValue(value any) any {
return value
}
}
+
+// hexDefault returns the hex single-value form of a binary or fixed default
+// given as bytes. It reports false for any other type or value, including a
+// default that is already a string.
+func hexDefault(typ Type, value any) (string, bool) {
+ switch typ.(type) {
+ case BinaryType, FixedType:
+ switch value := value.(type) {
+ case []byte:
+ return internal.EncodeDefaultBytes(value), true
+ case BinaryLiteral:
+ return internal.EncodeDefaultBytes(value), true
+ case FixedLiteral:
+ return internal.EncodeDefaultBytes(value), true
+ }
+ }
+
+ return "", false
+}
+
+// normalizeDefaults stores binary and fixed defaults in their hex single-value
+// form, the same form metadata JSON decodes to, so a schema holds no byte
+// slices that callers could share or change. When something is rewritten it
+// copies the fields and nested types on the path to that default and reports
+// true; otherwise it returns fields itself. The input is never modified.
+func normalizeDefaults(fields []NestedField) ([]NestedField, bool) {
+ var out []NestedField
+ for i, field := range fields {
+ normalized, ok := normalizeFieldDefaults(field)
+ if !ok {
+ continue
+ }
+ if out == nil {
+ out = slices.Clone(fields)
+ }
+ out[i] = normalized
+ }
+ if out == nil {
+ return fields, false
+ }
+
+ return out, true
+}
+
+func normalizeFieldDefaults(field NestedField) (NestedField, bool) {
+ changed := false
+ if value, ok := hexDefault(field.Type, field.InitialDefault); ok {
+ field.InitialDefault, changed = value, true
+ }
+ if value, ok := hexDefault(field.Type, field.WriteDefault); ok {
+ field.WriteDefault, changed = value, true
+ }
+ if typ, ok := normalizeTypeDefaults(field.Type); ok {
+ field.Type, changed = typ, true
+ }
+
+ return field, changed
+}
+
+func normalizeTypeDefaults(typ Type) (Type, bool) {
Review Comment:
I'd keep the copy-on-write walk. On a 200-field nested schema with no byte
defaults it makes 0 allocations, while `cloneFields` makes 201 (36 KB), and
every `NewSchema` call would pay that for defaults almost no schema has.
Composite defaults sharing the caller's data isn't specific to defaults either,
since `NewSchema` keeps the caller's field slice as is; #2062 makes
constructors copy their inputs and then removes `cloneField` / `cloneType`, so
I'd rather not build on them here.
--
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]
