Repository: jorgecarleitao/arrow2
Pull request #277 (2021-08-11 19:31:59)

Simplified IPC code.
Makes it more obvious why `unsafe` there is sound.

codecov[bot]: # [Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/277?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) Report
> Merging [#277](https://codecov.io/gh/jorgecarleitao/arrow2/pull/277?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) (70825fd) into [main](https://codecov.io/gh/jorgecarleitao/arrow2/commit/4c18c0ad88d24afa7854b3ef0584c357511ff9ce?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) (4c18c0a) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/jorgecarleitao/arrow2/pull/277/graphs/tree.svg?width=650&height=150&src=pr&token=AgyTF60R3D&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao)](https://codecov.io/gh/jorgecarleitao/arrow2/pull/277?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao)

```diff
@@           Coverage Diff           @@
##             main     #277   +/-   ##
=======================================
  Coverage   77.14%   77.15%           
=======================================
  Files         254      254           
  Lines       20648    20649    +1     
=======================================
+ Hits        15929    15931    +2     
+ Misses       4719     4718    -1     
```


| [Impacted Files](https://codecov.io/gh/jorgecarleitao/arrow2/pull/277?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) | Coverage Δ | |
|---|---|---|
| [src/io/ipc/read/read\_basic.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/277/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2lvL2lwYy9yZWFkL3JlYWRfYmFzaWMucnM=) | `88.50% <100.00%> (+0.13%)` | :arrow_up: |
| [src/compute/arithmetics/mod.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/277/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2NvbXB1dGUvYXJpdGhtZXRpY3MvbW9kLnJz) | `51.87% <0.00%> (+0.75%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/277?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/277?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao). Last update [4c18c0a...70825fd](https://codecov.io/gh/jorgecarleitao/arrow2/pull/277?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao).


Patch:
diff --git a/Cargo.toml b/Cargo.toml
index 0775ae6a8d..253e4ad7a7 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -33,7 +33,7 @@ serde_json = { version = "^1.0", features = ["preserve_order"], optional = true
 indexmap = { version = "^1.6", optional = true }
 
 # used to print columns in a nice columnar format
-comfy-table = { version = "4.0", optional = true }
+comfy-table = { version = "4.0", optional = true, default-features = false }
 
 flatbuffers = { version = "=2.0.0", optional = true }
 hex = { version = "^0.4", optional = true }
diff --git a/src/io/ipc/read/read_basic.rs b/src/io/ipc/read/read_basic.rs
index 421bf0d365..fd84edee75 100644
--- a/src/io/ipc/read/read_basic.rs
+++ b/src/io/ipc/read/read_basic.rs
@@ -10,18 +10,19 @@ use crate::{bitmap::Bitmap, buffer::MutableBuffer, types::NativeType};
 use super::super::compression;
 use super::super::gen;
 
-fn read_big_endian<T: NativeType, R: Read + Seek>(
+fn read_swapped<T: NativeType, R: Read + Seek>(
     reader: &mut R,
-    bytes: usize,
+    length: usize,
     buffer: &mut MutableBuffer<T>,
     is_little_endian: bool,
 ) -> Result<()> {
     // slow case where we must reverse bits
-    let mut slice = vec![0u8; bytes];
+    let mut slice = vec![0u8; length * std::mem::size_of::<T>()];
     reader.read_exact(&mut slice)?;
 
+    let chunks = slice.chunks_exact(std::mem::size_of::<T>());
     if !is_little_endian {
-        let chunks = slice.chunks_exact(std::mem::size_of::<T>());
+        // machine is little endian, file is big endian
         buffer
             .as_mut_slice()
             .iter_mut()
@@ -35,6 +36,9 @@ fn read_big_endian<T: NativeType, R: Read + Seek>(
                 Result::Ok(())
             })
             .unwrap();
+    } else {
+        // machine is big endian, file is little endian
+        todo!("reading little endian files from big endian machines not yet implemented.")
     }
     Ok(())
 }
@@ -42,10 +46,10 @@ fn read_big_endian<T: NativeType, R: Read + Seek>(
 fn read_uncompressed_buffer<T: NativeType, R: Read + Seek>(
     reader: &mut R,
     buffer_length: usize,
-    bytes: usize,
     length: usize,
     is_little_endian: bool,
 ) -> Result<MutableBuffer<T>> {
+    let bytes = length * std::mem::size_of::<T>();
     if bytes > buffer_length {
         return Err(ArrowError::Ipc(
             format!("The slots of the array times the physical size must \
@@ -68,11 +72,14 @@ fn read_uncompressed_buffer<T: NativeType, R: Read + Seek>(
         // fast case where we can just copy the contents as is
         unsafe {
             // transmute T to bytes.
-            let slice = std::slice::from_raw_parts_mut(buffer.as_mut_ptr() as *mut u8, bytes);
+            let slice = std::slice::from_raw_parts_mut(
+                buffer.as_mut_ptr() as *mut u8,
+                length * std::mem::size_of::<T>(),
+            );
             reader.read_exact(slice)?;
         }
     } else {
-        read_big_endian(reader, bytes, &mut buffer, is_little_endian)?;
+        read_swapped(reader, length, &mut buffer, is_little_endian)?;
     }
     Ok(buffer)
 }
@@ -144,18 +151,13 @@ pub fn read_buffer<T: NativeType, R: Read + Seek>(
 
     let buffer_length = buf.length() as usize;
 
-    let bytes = length * std::mem::size_of::<T>();
-
     if let Some(compression) = compression {
         Ok(
             read_compressed_buffer(reader, buffer_length, length, is_little_endian, compression)?
                 .into(),
         )
     } else {
-        Ok(
-            read_uncompressed_buffer(reader, buffer_length, bytes, length, is_little_endian)?
-                .into(),
-        )
+        Ok(read_uncompressed_buffer(reader, buffer_length, length, is_little_endian)?.into())
     }
 }
 


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 4655dd4afc..987589d9f3 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -271,7 +271,7 @@ jobs:
           export CARGO_HOME="/github/home/.cargo"
           export CARGO_TARGET_DIR="/github/home/target"
           # no need 
-          cargo build --no-default-features --features=merge_sort,io_ipc,io_csv,io_json,io_parquet --target wasm32-unknown-unknown
+          cargo build --no-default-features --features=merge_sort,io_ipc,io_csv,io_print,io_json,io_parquet --target wasm32-unknown-unknown
 
   linux-simd-test:
     name: SIMD