You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/12/02 16:50:20 UTC

[GitHub] [arrow] alamb commented on pull request #8784: WIP: ARROW-10674: [Rust] Fix BigDecimal to be little endian; Add IPC Reader/Writer for Decimal type to allow integration tests

alamb commented on pull request #8784:
URL: https://github.com/apache/arrow/pull/8784#issuecomment-737356705


   @sweb  -- it seems from the arrow definition that the endianness *may* be big or little endiain:
   https://github.com/apache/arrow/blob/master/format/Schema.fbs#L175-L178
   
   ```
   /// Exact decimal value represented as an integer value in two's
   /// complement. Currently only 128-bit (16-byte) and 256-bit (32-byte) integers
   /// are used. The representation uses the endianness indicated
   /// in the Schema.
   ```
   
   
   So I suggest at least validating the schema in the IPC message and error'ing with a "unimplemented" type error if it the schema is Big Endian. 
   
   The endianness can be checked:
   https://docs.rs/arrow/2.0.0/arrow/ipc/gen/Schema/struct.Schema.html#method.endianness
   
   Note that I still think this implementation (that now passes a new test) is still ok to merge in as is (as it is *more* correct for one case) but gracefully handling an unimplemented endianness would be better. 


----------------------------------------------------------------
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:
users@infra.apache.org