yevgenypats commented on code in PR #34454:
URL: https://github.com/apache/arrow/pull/34454#discussion_r1127408562
##########
go/arrow/datatype_extension.go:
##########
@@ -122,7 +124,9 @@ type ExtensionType interface {
// If the storage type is incorrect or something else is invalid with
the data this should
// return nil and an appropriate error.
Deserialize(storageType DataType, data string) (ExtensionType, error)
-
+ // this should return array.Builder interface but we cannot import due
to cycle import, so we use
+ // interface{} instead. At least for
+ NewBuilder(mem memory.Allocator, dt ExtensionType) interface{}
Review Comment:
On the first part, we can keep the same pattern, sure. But actually I think
this should 1) provide better performance, no reflection. 2) much better type
safety, and better developer experience as it is clear what should be the type
of this function (I also wanted to return Builder but had to fallback to
interface due to some cyclic import that didn't want to fix part of this PR).
Re 2nd thing, not sure I followed, can you share an example of what this
going to break? The only place that we use this function now is
[here](https://github.com/apache/arrow/pull/34454/files#diff-9a870facf93f4b8b367b13881398e3918d0627e348079d33017231d2b389d8bcR322)
and I fallback to the previous builder.
--
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]
