You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "SAY-5 (via GitHub)" <gi...@apache.org> on 2026/05/10 23:26:45 UTC
[PR] fix(avro): error on complex (non-nullable) unions instead of silently dropping (#777) [arrow-go]
SAY-5 opened a new pull request, #808:
URL: https://github.com/apache/arrow-go/pull/808
### Rationale for this change
Fixes #777. `ArrowSchemaFromAvro` previously silently dropped non-nullable Avro unions (e.g. `["int","string"]`) in both `arrowSchemafromAvro` and `iterateFields`. Output schemas were missing fields with no warning to the caller.
### What changes are included in this PR?
Panic with a clear error message at the two `case "union"` / `case *avro.UnionSchema` arms when the union is not a simple nullable. The package's existing `recover()` in `ArrowSchemaFromAvro` converts this into a returned error.
### Are these changes tested?
Yes — `TestComplexUnionReportsError` exercises a record with `["int","string"]` and asserts a non-nil error mentioning `union`.
### Are there any user-facing changes?
Callers that previously received an incomplete schema for a complex-union field now get an explicit error.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(avro): error on complex (non-nullable) unions instead of silently dropping (#777) [arrow-go]
Posted by "SAY-5 (via GitHub)" <gi...@apache.org>.
SAY-5 commented on PR #808:
URL: https://github.com/apache/arrow-go/pull/808#issuecomment-4432909117
I'd lean toward doing the Arrow union type support as a follow-up. This PR is intentionally narrow: it just turns the silent data-loss case (non-nullable unions being dropped) into a clear error, which is a safe incremental step. Mapping Avro unions onto `arrow.UnionType` (dense vs sparse, type-id assignment, the reader/writer plumbing) is a meatier change that deserves its own PR and tests. Happy to open a tracking issue for it if you'd like.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(avro): error on complex (non-nullable) unions instead of silently dropping (#777) [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #808:
URL: https://github.com/apache/arrow-go/pull/808#issuecomment-4443416445
Sounds good, please add a tracking issue for the union support and we can do that as a follow-up.
I'll merge this after the CI passes
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(avro): error on complex (non-nullable) unions instead of silently dropping (#777) [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #808:
URL: https://github.com/apache/arrow-go/pull/808
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(avro): error on complex (non-nullable) unions instead of silently dropping (#777) [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #808:
URL: https://github.com/apache/arrow-go/pull/808#discussion_r3228015107
##########
arrow/avro/schema.go:
##########
@@ -139,13 +139,16 @@ func arrowSchemafromAvro(n *schemaNode) {
arrowSchemafromAvro(c)
n.arrowField = buildArrowField(n, arrow.MapOf(arrow.BinaryTypes.String, c.arrowField.Type), c.arrowField.Metadata)
case "union":
- if n.schema.(*avro.UnionSchema).Nullable() {
- if len(n.schema.(*avro.UnionSchema).Types()) > 1 {
- n.schema = n.schema.(*avro.UnionSchema).Types()[1]
+ us := n.schema.(*avro.UnionSchema)
+ if us.Nullable() {
+ if len(us.Types()) > 1 {
+ n.schema = us.Types()[1]
n.union = true
n.nullable = true
arrowSchemafromAvro(n)
}
+ } else {
+ panic(fmt.Errorf("complex (non-nullable) avro union at '%v' is not supported", n.schemaPath()))
Review Comment:
Should we add support and use the arrow union type? We could also do this as a follow up instead.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org