matan129 commented on code in PR #850:
URL: https://github.com/apache/arrow-go/pull/850#discussion_r3390568134
##########
arrow/avro/reader_types.go:
##########
@@ -594,6 +594,10 @@ func appendBinaryData(b *array.BinaryBuilder, data
interface{}) {
switch dt := data.(type) {
case nil:
b.AppendNull()
+ case []byte:
+ b.Append(dt)
Review Comment:
Done in 6a7b49b. Went with an error return rather than a panic since the
plumbing already exists: `appendFunc` returns `error` and `loadDatum`
propagates it to `OCFReader.Err()`, same as the decimal appenders. Both
`appendBinaryData` and `appendStringData` now error on any type outside what
hamba produces, with a test covering the error paths.
##########
arrow/avro/reader_types.go:
##########
@@ -594,6 +594,10 @@ func appendBinaryData(b *array.BinaryBuilder, data
interface{}) {
switch dt := data.(type) {
case nil:
b.AppendNull()
+ case []byte:
+ b.Append(dt)
+ case string:
+ b.Append([]byte(dt))
Review Comment:
Dropped it in 6a7b49b. With the default now returning an error, a string
reaching a binary column fails loudly instead of being silently coerced, which
seems preferable to keeping the dead case.
##########
arrow/avro/reader_types.go:
##########
@@ -594,6 +594,10 @@ func appendBinaryData(b *array.BinaryBuilder, data
interface{}) {
switch dt := data.(type) {
case nil:
b.AppendNull()
+ case []byte:
+ b.Append(dt)
+ case string:
+ b.Append([]byte(dt))
case map[string]any:
switch ct := dt["bytes"].(type) {
Review Comment:
Done in 6a7b49b — the union branch now uses a typed `case []byte` like
`appendFixedSizeBinaryData`, and anything else falls into the new error default
rather than panicking.
--
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]
