alamb commented on a change in pull request #8784:
URL: https://github.com/apache/arrow/pull/8784#discussion_r535138494
##########
File path: rust/arrow/src/ipc/convert.rs
##########
@@ -97,6 +97,12 @@ pub fn fb_to_schema(fb: ipc::Schema) -> Schema {
let len = c_fields.len();
for i in 0..len {
let c_field: ipc::Field = c_fields.get(i);
+ match c_field.type_type() {
Review comment:
I see the problem -- yes, there since the `endianness` is on the shema
object, not the field, since the field is all that is passed around there is no
way to know what the details of the schema are.
I personally think this code is fine, if a bit un-indeal and could be
cleaned up in the future. My only worry is that it would get lost / broken
during such cleanup
What would you think about adding a test that triggers the error? Then we
could be sure that any future cleanups will not break the check?
##########
File path: rust/arrow/src/ipc/convert.rs
##########
@@ -97,6 +97,12 @@ pub fn fb_to_schema(fb: ipc::Schema) -> Schema {
let len = c_fields.len();
for i in 0..len {
let c_field: ipc::Field = c_fields.get(i);
+ match c_field.type_type() {
Review comment:
I see the problem -- yes, there since the `endianness` is on the shema
object, not the field, since the field is all that is passed around there is no
way to know what the details of the schema are.
I personally think this code is fine, if a bit un-indeal and could be
cleaned up in the future. My only worry is that it would get lost / broken
during such cleanup
What would you think about adding a test that triggers the error? Then we
could be sure that any future cleanups will not break the check?
Thanks again @sweb
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
