Revanth14 commented on issue #2062: URL: https://github.com/apache/iceberg-go/issues/2062#issuecomment-6625903660
> I'd like to claim deep immutability of schema types, the whole series. > > Plan, in order: > > 1. **JSON marshalers.** `StructType` has no `UnmarshalJSON` and decodes through the exported `FieldList` tag. `ListType` / `MapType` marshal through an alias of their exported fields. I'll add explicit marshalers for all three, with round-trip tests that fail once a field is unexported. > 2. **`[]byte` defaults.** Store binary and fixed defaults as hex strings, so copying a `NestedField` no longer shares a backing array. > 3. **Constructors and accessors, additive.** Add `NewStructType` / `NewListType` / `NewMapType` and the accessors while the fields are still exported. Constructors keep returning `*StructType` etc., so `Equals` and the type switches don't change. `Fields()` stays `[]NestedField`, as a shallow copy. > 4. **Call-site migration.** A `go/ast` rewriter rather than `gofmt -r`, because the literals use keyed fields in varying orders. The release note will carry the same recipe. > 5. **Unexport and remove.** Unexport the fields, then drop `cloneField(s)` / `cloneType`, the `Ref` accessors and `internal.SchemaRef` / `internal.PartitionSpecRef`. > > Steps 1–3 don't conflict with much, so I'd start them now. Only 4–5 need the announced freeze, after [#2357](https://github.com/apache/iceberg-go/pull/2357), [#2083](https://github.com/apache/iceberg-go/issues/2083), [#2349](https://github.com/apache/iceberg-go/pull/2349) and the SQL view alignment land. > > Scope: I'll include `Schema.IdentifierFieldIDs`, and keep `PartitionField.SourceIDs` / `SortField.SourceIDs` exported and copied on read, per [@laskoviymishka](https://github.com/laskoviymishka)'s comment above, unless [@zeroshade](https://github.com/zeroshade) decides otherwise. A correction to step 3 of my plan: the `ListType` / `MapType` accessors can't be added while the fields are exported. `Element`, `ElementID`, `KeyType`, `ValueType` and the rest are field names, and Go doesn't allow a method with the same name. The same goes for `Schema.IdentifierFieldIDs`. So step 3 adds only the constructors plus `StructType.NumFields()` / `Field(i)`. The same-name accessors come in the unexport PR, together with the read rewrite (about 150 non-test reads). Since the literal migration doesn't break anything while the fields are exported, I'd like to land it package by package before the freeze (`table/` after #2357 and #2083), leaving only the unexport PR under the freeze. @zeroshade, does that work for you, or do you prefer one migration PR during the freeze? -- 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]
