zeroshade commented on code in PR #35307:
URL: https://github.com/apache/arrow/pull/35307#discussion_r1175779742
##########
go/arrow/schema.go:
##########
@@ -194,7 +194,11 @@ func (sc *Schema) WithEndianness(e endian.Endianness)
*Schema {
func (sc *Schema) Endianness() endian.Endianness { return sc.endianness }
func (sc *Schema) IsNativeEndian() bool { return sc.endianness ==
endian.NativeEndian }
func (sc *Schema) Metadata() Metadata { return sc.meta }
-func (sc *Schema) Fields() []Field { return sc.fields }
+func (sc *Schema) Fields() []Field {
+ fields := make([]Field, len(sc.fields))
+ copy(fields, sc.fields)
+ return fields
+}
Review Comment:
`Metadata` only ends up as a copy because we return the metadata object by
value which just contains two slices in it which get passed by reference to the
new copy. but appending to a slice in Go doesn't change *other* references to
that slice.
For example, if you append to the list of fields you get back from
`Fields()` it won't actually modify the `Fields()` slice internally to the
Schema. But because slices are reference types in Go, modifying the fields
*inside* the slice will manipulate internal state. Sometimes this can be a good
thing (like updating a nullable flag / updating metadata for a field) other
times it allows users to do bad things.
I guess for now we can update this to be a copy and if anyone has a
performance issue with this (not really likely, but possible) we can address it
at that point.
--
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]
