Repository: jorgecarleitao/arrow2
Pull request #227 (2021-07-25 14:50:58)

Fix generic parameter in lexsort usage in benches

codecov[bot]: # [Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/227?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) Report
> Merging [#227](https://codecov.io/gh/jorgecarleitao/arrow2/pull/227?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) (0eab705) into [main](https://codecov.io/gh/jorgecarleitao/arrow2/commit/7dd1cff6821f70e3b6a499f55eaef42b785284cd?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) (7dd1cff) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/jorgecarleitao/arrow2/pull/227/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/227?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     #227   +/-   ##
=======================================
  Coverage   76.93%   76.93%           
=======================================
  Files         229      229           
  Lines       19529    19529           
=======================================
  Hits        15025    15025           
  Misses       4504     4504           
```



------

[Continue to review full report at Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/227?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/227?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao). Last update [7dd1cff...0eab705](https://codecov.io/gh/jorgecarleitao/arrow2/pull/227?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).

ritchie46: This is a fix for the benches that don't run on main.
sundy-li: > This is a fix for the benches that don't run on main.

Thanks for the advice, but better to fix this if we need run some bench tests.

Patch:
diff --git a/README.md b/README.md
index b958fb4712..d40f2baff1 100644
--- a/README.md
+++ b/README.md
@@ -38,6 +38,7 @@ To not run them, pass `ARROW2_IGNORE_PARQUET` to the tests (the tests will be ma
 ```bash
 git clone git@github.com:jorgecarleitao/arrow2.git
 cd arrow2
+git submodule update --init
 ARROW2_IGNORE_PARQUET= cargo test
 ```
 
diff --git a/benches/sort_kernel.rs b/benches/sort_kernel.rs
index ed48c4c8e4..30d89c441c 100644
--- a/benches/sort_kernel.rs
+++ b/benches/sort_kernel.rs
@@ -35,7 +35,7 @@ fn bench_lexsort(arr_a: &dyn Array, array_b: &dyn Array) {
         },
     ];
 
-    criterion::black_box(lexsort(&columns, None).unwrap());
+    criterion::black_box(lexsort::<u32>(&columns, None).unwrap());
 }
 
 fn bench_sort(arr_a: &dyn Array) {
diff --git a/src/array/fixed_size_list/mod.rs b/src/array/fixed_size_list/mod.rs
index 2b5c392303..df5877162d 100644
--- a/src/array/fixed_size_list/mod.rs
+++ b/src/array/fixed_size_list/mod.rs
@@ -133,4 +133,8 @@ unsafe impl ToFfi for FixedSizeListArray {
     fn offset(&self) -> usize {
         self.offset
     }
+
+    fn children(&self) -> Vec<Arc<dyn Array>> {
+        vec![self.values().clone()]
+    }
 }
diff --git a/src/array/growable/list.rs b/src/array/growable/list.rs
index 729d3c4404..311b79aacd 100644
--- a/src/array/growable/list.rs
+++ b/src/array/growable/list.rs
@@ -72,7 +72,7 @@ impl<'a, O: Offset> GrowableList<'a, O> {
     pub fn new(arrays: &[&'a dyn Array], mut use_validity: bool, capacity: usize) -> Self {
         // if any of the arrays has nulls, insertions from any array requires setting bits
         // as there is at least one array with nulls.
-        if arrays.iter().any(|array| array.null_count() > 0) {
+        if use_validity || arrays.iter().any(|array| array.null_count() > 0) {
             use_validity = true;
         };
 
diff --git a/src/ffi/array.rs b/src/ffi/array.rs
index 3a74cc5187..0929aaebc9 100644
--- a/src/ffi/array.rs
+++ b/src/ffi/array.rs
@@ -81,6 +81,7 @@ pub fn try_from<A: ArrowArrayRef>(array: A) -> Result<Box<dyn Array>> {
 mod tests {
     use super::*;
     use crate::array::*;
+    use crate::datatypes::TimeUnit;
     use crate::{error::Result, ffi};
     use std::sync::Arc;
 
@@ -146,6 +147,15 @@ mod tests {
         test_round_trip(data)
     }
 
+    #[test]
+    fn test_timestamp_tz() -> Result<()> {
+        let data = Int64Array::from(&vec![Some(2), None, None]).to(DataType::Timestamp(
+            TimeUnit::Second,
+            Some("UTC".to_string()),
+        ));
+        test_round_trip(data)
+    }
+
     #[test]
     fn test_large_binary() -> Result<()> {
         let data =
diff --git a/src/ffi/ffi.rs b/src/ffi/ffi.rs
index 05569d7875..f4c1f77669 100644
--- a/src/ffi/ffi.rs
+++ b/src/ffi/ffi.rs
@@ -29,7 +29,7 @@ use crate::{
         bytes::{Bytes, Deallocation},
         Buffer,
     },
-    datatypes::{DataType, Field, TimeUnit},
+    datatypes::{DataType, Field, IntervalUnit, TimeUnit},
     error::{ArrowError, Result},
     types::NativeType,
 };
@@ -200,6 +200,12 @@ fn to_field(schema: &Ffi_ArrowSchema) -> Result<Field> {
         "ttm" => DataType::Time32(TimeUnit::Millisecond),
         "ttu" => DataType::Time64(TimeUnit::Microsecond),
         "ttn" => DataType::Time64(TimeUnit::Nanosecond),
+        "tDs" => DataType::Duration(TimeUnit::Second),
+        "tDm" => DataType::Duration(TimeUnit::Millisecond),
+        "tDu" => DataType::Duration(TimeUnit::Microsecond),
+        "tDn" => DataType::Duration(TimeUnit::Nanosecond),
+        "tiM" => DataType::Interval(IntervalUnit::YearMonth),
+        "tiD" => DataType::Interval(IntervalUnit::DayTime),
         "+l" => {
             let child = schema.child(0);
             DataType::List(Box::new(to_field(child)?))
@@ -215,10 +221,43 @@ fn to_field(schema: &Ffi_ArrowSchema) -> Result<Field> {
             DataType::Struct(children)
         }
         other => {
-            return Err(ArrowError::Ffi(format!(
-                "The datatype \"{}\" is still not supported in Rust implementation",
-                other
-            )))
+            let parts = other.split(':').collect::<Vec<_>>();
+            if parts.len() == 2 && parts[0] == "tss" {
+                DataType::Timestamp(TimeUnit::Second, Some(parts[1].to_string()))
+            } else if parts.len() == 2 && parts[0] == "tsm" {
+                DataType::Timestamp(TimeUnit::Millisecond, Some(parts[1].to_string()))
+            } else if parts.len() == 2 && parts[0] == "tsu" {
+                DataType::Timestamp(TimeUnit::Microsecond, Some(parts[1].to_string()))
+            } else if parts.len() == 2 && parts[0] == "tsn" {
+                DataType::Timestamp(TimeUnit::Nanosecond, Some(parts[1].to_string()))
+            } else if parts.len() == 2 && parts[0] == "d" {
+                let parts = parts[1].split(',').collect::<Vec<_>>();
+                if parts.len() < 2 || parts.len() > 3 {
+                    return Err(ArrowError::Ffi(
+                        "Decimal must contain 2 or 3 comma-separated values".to_string(),
+                    ));
+                };
+                if parts.len() == 3 {
+                    let bit_width = parts[0].parse::<usize>().map_err(|_| {
+                        ArrowError::Ffi("Decimal bit width is not a valid integer".to_string())
+                    })?;
+                    if bit_width != 128 {
+                        return Err(ArrowError::Ffi("Decimal256 is not supported".to_string()));
+                    }
+                }
+                let precision = parts[0].parse::<usize>().map_err(|_| {
+                    ArrowError::Ffi("Decimal precision is not a valid integer".to_string())
+                })?;
+                let scale = parts[1].parse::<usize>().map_err(|_| {
+                    ArrowError::Ffi("Decimal scale is not a valid integer".to_string())
+                })?;
+                DataType::Decimal(precision, scale)
+            } else {
+                return Err(ArrowError::Ffi(format!(
+                    "The datatype \"{}\" is still not supported in Rust implementation",
+                    other
+                )));
+            }
         }
     };
     Ok(Field::new(schema.name(), data_type, schema.nullable()))
@@ -250,15 +289,33 @@ fn to_format(data_type: &DataType) -> Result<String> {
         DataType::Time32(TimeUnit::Millisecond) => "ttm",
         DataType::Time64(TimeUnit::Microsecond) => "ttu",
         DataType::Time64(TimeUnit::Nanosecond) => "ttn",
+        DataType::Duration(TimeUnit::Second) => "tDs",
+        DataType::Duration(TimeUnit::Millisecond) => "tDm",
+        DataType::Duration(TimeUnit::Microsecond) => "tDu",
+        DataType::Duration(TimeUnit::Nanosecond) => "tDn",
+        DataType::Interval(IntervalUnit::YearMonth) => "tiM",
+        DataType::Interval(IntervalUnit::DayTime) => "tiD",
+        DataType::Timestamp(unit, tz) => {
+            let unit = match unit {
+                TimeUnit::Second => "s",
+                TimeUnit::Millisecond => "m",
+                TimeUnit::Microsecond => "u",
+                TimeUnit::Nanosecond => "n",
+            };
+            return Ok(format!(
+                "ts{}:{}",
+                unit,
+                tz.as_ref().map(|x| x.as_ref()).unwrap_or("")
+            ));
+        }
+        DataType::Decimal(precision, scale) => return Ok(format!("d:{},{}", precision, scale)),
         DataType::List(_) => "+l",
         DataType::LargeList(_) => "+L",
         DataType::Struct(_) => "+s",
-        z => {
-            return Err(ArrowError::Ffi(format!(
-                "The datatype \"{:?}\" is still not supported in Rust implementation",
-                z
-            )))
-        }
+        DataType::FixedSizeBinary(size) => return Ok(format!("w{}", size)),
+        DataType::FixedSizeList(_, size) => return Ok(format!("+w:{}", size)),
+        DataType::Union(_) => todo!(),
+        _ => todo!(),
     }
     .to_string())
 }


Test patch:
diff --git a/arrow-pyarrow-integration-testing/tests/test_sql.py b/arrow-pyarrow-integration-testing/tests/test_sql.py
index c3b6e7843a..e6ec3223d0 100644
--- a/arrow-pyarrow-integration-testing/tests/test_sql.py
+++ b/arrow-pyarrow-integration-testing/tests/test_sql.py
@@ -17,6 +17,7 @@
 # under the License.
 
 import unittest
+import decimal
 
 import pyarrow
 import arrow_pyarrow_integration_testing
@@ -96,6 +97,19 @@ class TestCase(unittest.TestCase):
         c = pyarrow.array(["a", None, "ccc"])
         self.assertEqual(b, c)
 
+    def test_decimal_roundtrip(self):
+        """
+        Python -> Rust -> Python
+        """
+        data = [
+            round(decimal.Decimal(722.82), 2),
+            round(decimal.Decimal(-934.11), 2),
+            None
+        ]
+        a = pyarrow.array(data, pyarrow.decimal128(5, 2))
+        b = arrow_pyarrow_integration_testing.round_trip(a)
+        self.assertEqual(a, b)
+
     def test_string_python(self):
         """
         Python -> Rust -> Python