You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "NickCrews (via GitHub)" <gi...@apache.org> on 2024/09/19 01:54:17 UTC

[I] structs: Support casting nullable fields to non-nullable if there are no null values [arrow-go]

NickCrews opened a new issue, #133:
URL: https://github.com/apache/arrow-go/issues/133

   This solves https://github.com/apache/arrow/issues/33592
   
   This is a sister PR to https://github.com/apache/arrow/pull/43782, I was told to open a PR here to make this change on the Go side.
   
   I need a few pointers though if you want me to do the implementation. Could you give me a high-level summary of what needs to happen? I did an initial exploration and had a few notes/questions:
   - I went looking for an equivalent nullable to non-nullable cast test for non-struct arrays, but couldn't find any. Is this possibly not tested? Or do I misunderstand the arrow spec, and all datatypes EXCEPT struct fields are nullable, so there is no eg non-nullable uint16?
   - More philosophically, is this behavior actually in the arrow spec? Or is the spec just about the in-memory format, and implementations are free to define their compute/casting behavior as desired? It seems funny/non-ideal to me that it is possible for the go implementation of this casting to drift away from the c++ implementation. I would think it would be better if there was some test that verified they both had the same behavior?
   - Same thing for the actual implementation. I wanted to use these as examples to work from.
   - Note for myself/fixer: [here is the struct test](https://github.com/apache/arrow-go/blob/12482ca3eb1cd4da21c45325e174f1dac871281b/arrow/compute/cast_test.go#L2639) that will need to get updated


-- 
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: issues-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] structs: Support casting nullable fields to non-nullable if there are no null values [arrow-go]

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade closed issue #133: structs: Support casting nullable fields to non-nullable if there are no null values
URL: https://github.com/apache/arrow-go/issues/133


-- 
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: issues-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] structs: Support casting nullable fields to non-nullable if there are no null values [arrow-go]

Posted by "0xderek (via GitHub)" <gi...@apache.org>.
0xderek commented on issue #133:
URL: https://github.com/apache/arrow-go/issues/133#issuecomment-2636800773

   I came across a relevant issue regarding field nullability.
   
   I am trying to build a `Record` and later send it to other system in ipc format. I use `array.NewRecordBuilder(allocator, schema)` to get the record builder, but the `nullable` property of the fields in the schema is not honored. It works without error to specify a non-nullable field in the schema, append nulls to the field and get serialized bytes.
   
   Personally, I think this is a bit confusing and error-prone. It's better if error is returned somewhere (preferably when `NewRecord` is called since there is a call to `validate()` inside).


-- 
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: [I] structs: Support casting nullable fields to non-nullable if there are no null values [arrow-go]

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on issue #133:
URL: https://github.com/apache/arrow-go/issues/133#issuecomment-2366859423

   > More philosophically, is this behavior actually in the arrow spec? Or is the spec just about the in-memory format, and implementations are free to define their compute/casting behavior as desired? It seems funny/non-ideal to me that it is possible for the go implementation of this casting to drift away from the c++ implementation. I would think it would be better if there was some test that verified they both had the same behavior?
   
   This is the case, the spec *just* defines the in-memory format. Anything implementing compute and casting behavior is outside the scope of the spec itself as different compute engines are free to have different behaviors. For a spec attempting to create standardized behavior on compute functionality, there is https://substrait.io 
   
   That said, I agree the ideal would be for the go implementation to follow the C++ implementation where possible for these things.
   
   > I went looking for an equivalent nullable to non-nullable cast test for non-struct arrays, but couldn't find any. Is this possibly not tested? Or do I misunderstand the arrow spec, and all datatypes EXCEPT struct fields are nullable, so there is no eg non-nullable uint16?
   
   By the Arrow spec, "nullability" is a feature of "fields" as it is simply a piece of metadata to inform consumers of data. So you can only mark something as non-nullable by way of a schema (i.e. top-level columns in a record batch can be marked non-nullable via their fields in the schema) or by virtue of it being a nested field of some other type (i.e. struct, list, map, etc. type) which contains fields to describe its children.
   
   So you can have a non-nullable uint16, if the schema/parent struct/parent list type defines the column field as non-nullable. Since casting doesn't accept record batches, only Array or ChunkedArray, the only cases where you might run into this nullability issue are in the child fields of these nested types. Since we do provide factories for constructing lists with non-nullable element fields, we probably should add tests to verify we handle nullability/non-nullability for lists/maps/etc.
   
   


-- 
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