Repository: jorgecarleitao/arrow2
Pull request #1026 (2022-05-30 20:47:26)

Fixed error in consuming sliced `FixedSizedBinary` from c data interface (FFI)
The (fixed) size of the array must be taken into account when offsetting the values buffer.

This resulted in reading less elements than required (i.e. it would never cause a read out of bounds).


codecov[bot]: # [Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1026?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) Report
> Merging [#1026](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1026?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) (b18896c) into [main](https://codecov.io/gh/jorgecarleitao/arrow2/commit/266fbbe9e307b94a6006695b1c8654510e191804?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) (266fbbe) will **increase** coverage by `0.23%`.
> The diff coverage is `67.64%`.

```diff
@@            Coverage Diff             @@
##             main    #1026      +/-   ##
==========================================
+ Coverage   71.62%   71.86%   +0.23%     
==========================================
  Files         359      359              
  Lines       19907    19917      +10     
==========================================
+ Hits        14259    14313      +54     
+ Misses       5648     5604      -44     
```


| [Impacted Files](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1026?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) | Coverage Δ | |
|---|---|---|
| [src/ffi/array.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2ZmaS9hcnJheS5ycw==) | `86.62% <ø> (+2.54%)` | :arrow_up: |
| [src/bitmap/mutable.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2JpdG1hcC9tdXRhYmxlLnJz) | `89.16% <66.66%> (-1.10%)` | :arrow_down: |
| [src/ffi/schema.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2ZmaS9zY2hlbWEucnM=) | `75.16% <100.00%> (+6.87%)` | :arrow_up: |
| [src/compute/arithmetics/time.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2NvbXB1dGUvYXJpdGhtZXRpY3MvdGltZS5ycw==) | `25.68% <0.00%> (-0.92%)` | :arrow_down: |
| [src/array/list/mod.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L2xpc3QvbW9kLnJz) | `70.79% <0.00%> (+0.88%)` | :arrow_up: |
| [src/array/mod.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L21vZC5ycw==) | `68.08% <0.00%> (+1.06%)` | :arrow_up: |
| [src/bitmap/utils/slice\_iterator.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2JpdG1hcC91dGlscy9zbGljZV9pdGVyYXRvci5ycw==) | `87.93% <0.00%> (+1.72%)` | :arrow_up: |
| [src/array/fixed\_size\_binary/mod.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1026/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L2ZpeGVkX3NpemVfYmluYXJ5L21vZC5ycw==) | `90.29% <0.00%> (+1.94%)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1026/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1026?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/1026?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao). Last update [266fbbe...b18896c](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1026?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/src/ffi/array.rs b/src/ffi/array.rs
index 76df63ceb5..5ebdf5d7fc 100644
--- a/src/ffi/array.rs
+++ b/src/ffi/array.rs
@@ -243,6 +243,13 @@ fn buffer_offset(array: &ArrowArray, data_type: &DataType, i: usize) -> usize {
     use PhysicalType::*;
     match (data_type.to_physical_type(), i) {
         (LargeUtf8, 2) | (LargeBinary, 2) | (Utf8, 2) | (Binary, 2) => 0,
+        (FixedSizeBinary, 1) => {
+            if let DataType::FixedSizeBinary(size) = data_type.to_logical_type() {
+                (array.offset as usize) * *size
+            } else {
+                unreachable!()
+            }
+        }
         _ => array.offset as usize,
     }
 }


Test patch:
diff --git a/tests/it/ffi/data.rs b/tests/it/ffi/data.rs
index 63a4489a0d..39c9400114 100644
--- a/tests/it/ffi/data.rs
+++ b/tests/it/ffi/data.rs
@@ -81,6 +81,18 @@ fn u32() -> Result<()> {
     test_round_trip(data)
 }
 
+#[test]
+fn decimal() -> Result<()> {
+    let data = Int128Array::from_slice(&[1, 0, 2, 0]);
+    test_round_trip(data)
+}
+
+#[test]
+fn decimal_nullable() -> Result<()> {
+    let data = Int128Array::from(&[Some(1), None, Some(2), None]);
+    test_round_trip(data)
+}
+
 #[test]
 fn timestamp_tz() -> Result<()> {
     let data = Int64Array::from(&vec![Some(2), None, None]).to(DataType::Timestamp(
@@ -128,6 +140,26 @@ fn large_binary() -> Result<()> {
     test_round_trip(data)
 }
 
+#[test]
+fn fixed_size_binary() -> Result<()> {
+    let data = FixedSizeBinaryArray::new(
+        DataType::FixedSizeBinary(2),
+        vec![1, 2, 3, 4, 5, 6].into(),
+        None,
+    );
+    test_round_trip(data)
+}
+
+#[test]
+fn fixed_size_binary_nullable() -> Result<()> {
+    let data = FixedSizeBinaryArray::new(
+        DataType::FixedSizeBinary(2),
+        vec![1, 2, 3, 4, 5, 6].into(),
+        Some([true, true, false].into()),
+    );
+    test_round_trip(data)
+}
+
 #[test]
 fn list() -> Result<()> {
     let data = vec![
@@ -144,6 +176,22 @@ fn list() -> Result<()> {
     test_round_trip(array)
 }
 
+#[test]
+fn large_list() -> Result<()> {
+    let data = vec![
+        Some(vec![Some(1i32), Some(2), Some(3)]),
+        None,
+        Some(vec![Some(4), None, Some(6)]),
+    ];
+
+    let mut array = MutableListArray::<i64, MutablePrimitiveArray<i32>>::new();
+    array.try_extend(data)?;
+
+    let array: ListArray<i64> = array.into();
+
+    test_round_trip(array)
+}
+
 #[test]
 fn fixed_list() -> Result<()> {
     let data = vec![
@@ -231,7 +279,11 @@ fn schema() -> Result<()> {
 fn extension() -> Result<()> {
     let field = Field::new(
         "a",
-        DataType::Extension("a".to_string(), Box::new(DataType::Int32), None),
+        DataType::Extension(
+            "a".to_string(),
+            Box::new(DataType::Int32),
+            Some("bla".to_string()),
+        ),
         true,
     );
     test_round_trip_schema(field)