Repository: jorgecarleitao/arrow2
Pull request #1090 (2022-06-21 05:38:01)

Accept decoding parquet's `i64` into `u32` written by `pyarrow`
Despite [parquet's statement](https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#unsigned-integers) that `uint32` are encoded as `i32`, `pyarrow` writes `UInt32` columns in `v1` files in parquet's `i64` physical type without logical annotations but with an arrow schema declared as `UInt32`.

This PR makes us accept this configuration when reading parquet (from `i64` to `u32`).

Thanks @mhtrinhLIC for reporting this at https://github.com/pola-rs/polars/issues/3754

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

```diff
@@            Coverage Diff             @@
##             main    #1090      +/-   ##
==========================================
+ Coverage   81.07%   81.14%   +0.06%     
==========================================
  Files         367      367              
  Lines       35309    35503     +194     
==========================================
+ Hits        28628    28808     +180     
- Misses       6681     6695      +14     
```


| [Impacted Files](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1090?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) | Coverage Δ | |
|---|---|---|
| [src/io/parquet/read/deserialize/mod.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1090/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2lvL3BhcnF1ZXQvcmVhZC9kZXNlcmlhbGl6ZS9tb2QucnM=) | `53.95% <0.00%> (-2.33%)` | :arrow_down: |
| [src/io/parquet/read/statistics/mod.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1090/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2lvL3BhcnF1ZXQvcmVhZC9zdGF0aXN0aWNzL21vZC5ycw==) | `92.33% <37.50%> (-1.35%)` | :arrow_down: |
| [src/io/parquet/read/deserialize/simple.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1090/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2lvL3BhcnF1ZXQvcmVhZC9kZXNlcmlhbGl6ZS9zaW1wbGUucnM=) | `54.91% <72.22%> (+0.10%)` | :arrow_up: |
| [src/io/ipc/read/reader.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1090/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2lvL2lwYy9yZWFkL3JlYWRlci5ycw==) | `95.80% <0.00%> (+0.38%)` | :arrow_up: |
| [src/bitmap/mutable.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1090/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2JpdG1hcC9tdXRhYmxlLnJz) | `98.17% <0.00%> (+0.65%)` | :arrow_up: |
| [src/array/binary/mod.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1090/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L2JpbmFyeS9tb2QucnM=) | `90.04% <0.00%> (+1.24%)` | :arrow_up: |
| [src/bitmap/immutable.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1090/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2JpdG1hcC9pbW11dGFibGUucnM=) | `85.71% <0.00%> (+2.69%)` | :arrow_up: |
| [src/chunk.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1090/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2NodW5rLnJz) | `90.47% <0.00%> (+7.14%)` | :arrow_up: |
| [src/io/ipc/read/array/boolean.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1090/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2lvL2lwYy9yZWFkL2FycmF5L2Jvb2xlYW4ucnM=) | `97.91% <0.00%> (+8.33%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1090?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/1090?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao). Last update [d1ab4ef...e0ec381](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1090?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/io/parquet/read/deserialize/mod.rs b/src/io/parquet/read/deserialize/mod.rs
index 433d807baf..d4a7de6ee8 100644
--- a/src/io/parquet/read/deserialize/mod.rs
+++ b/src/io/parquet/read/deserialize/mod.rs
@@ -188,14 +188,29 @@ where
         }
         UInt32 => {
             init.push(InitNested::Primitive(field.is_nullable));
-            types.pop();
-            primitive::iter_to_arrays_nested(
-                columns.pop().unwrap(),
-                init,
-                field.data_type().clone(),
-                chunk_size,
-                |x: i32| x as u32,
-            )
+            let type_ = types.pop().unwrap();
+            match type_.physical_type {
+                PhysicalType::Int32 => primitive::iter_to_arrays_nested(
+                    columns.pop().unwrap(),
+                    init,
+                    field.data_type().clone(),
+                    chunk_size,
+                    |x: i32| x as u32,
+                ),
+                // some implementations of parquet write arrow's u32 into i64.
+                PhysicalType::Int64 => primitive::iter_to_arrays_nested(
+                    columns.pop().unwrap(),
+                    init,
+                    field.data_type().clone(),
+                    chunk_size,
+                    |x: i64| x as u32,
+                ),
+                other => {
+                    return Err(Error::nyi(format!(
+                        "Deserializing UInt32 from {other:?}'s parquet"
+                    )))
+                }
+            }
         }
         UInt64 => {
             init.push(InitNested::Primitive(field.is_nullable));
diff --git a/src/io/parquet/read/deserialize/simple.rs b/src/io/parquet/read/deserialize/simple.rs
index 6d3026c18b..8a8ef653da 100644
--- a/src/io/parquet/read/deserialize/simple.rs
+++ b/src/io/parquet/read/deserialize/simple.rs
@@ -82,12 +82,27 @@ pub fn page_iter_to_arrays<'a, I: 'a + DataPages>(
             chunk_size,
             |x: i32| x as u16,
         ))),
-        UInt32 => dyn_iter(iden(primitive::Iter::new(
-            pages,
-            data_type,
-            chunk_size,
-            |x: i32| x as u32,
-        ))),
+        UInt32 => match physical_type {
+            PhysicalType::Int32 => dyn_iter(iden(primitive::Iter::new(
+                pages,
+                data_type,
+                chunk_size,
+                |x: i32| x as u32,
+            ))),
+            // some implementations of parquet write arrow's u32 into i64.
+            PhysicalType::Int64 => dyn_iter(iden(primitive::Iter::new(
+                pages,
+                data_type,
+                chunk_size,
+                |x: i64| x as u32,
+            ))),
+            other => {
+                return Err(Error::NotYetImplemented(format!(
+                    "Reading uin32 from {:?}-encoded parquet still not implemented",
+                    other
+                )))
+            }
+        },
         Int8 => dyn_iter(iden(primitive::Iter::new(
             pages,
             data_type,
diff --git a/src/io/parquet/read/statistics/mod.rs b/src/io/parquet/read/statistics/mod.rs
index 4f3eaeff7b..540dac4df9 100644
--- a/src/io/parquet/read/statistics/mod.rs
+++ b/src/io/parquet/read/statistics/mod.rs
@@ -410,7 +410,17 @@ fn push(
         }
         UInt8 => primitive::push(from, min, max, |x: i32| Ok(x as u8)),
         UInt16 => primitive::push(from, min, max, |x: i32| Ok(x as u16)),
-        UInt32 => primitive::push(from, min, max, |x: i32| Ok(x as u32)),
+        UInt32 => match physical_type {
+            // some implementations of parquet write arrow's u32 into i64.
+            ParquetPhysicalType::Int64 => primitive::push(from, min, max, |x: i64| Ok(x as u32)),
+            ParquetPhysicalType::Int32 => primitive::push(from, min, max, |x: i32| Ok(x as u32)),
+            other => {
+                return Err(Error::NotYetImplemented(format!(
+                    "Can't decode UInt32 type from parquet type {:?}",
+                    other
+                )))
+            }
+        },
         Int32 => primitive::push(from, min, max, |x: i32| Ok(x as i32)),
         Int64 | Date64 | Time64(_) | Duration(_) => {
             primitive::push(from, min, max, |x: i64| Ok(x as i64))


Test patch:
diff --git a/tests/it/io/parquet/read.rs b/tests/it/io/parquet/read.rs
index 4e426ef5ba..8ab783dbab 100644
--- a/tests/it/io/parquet/read.rs
+++ b/tests/it/io/parquet/read.rs
@@ -130,11 +130,15 @@ fn v1_timestamp_ms_nullable() -> Result<()> {
 }
 
 #[test]
-#[ignore] // pyarrow issue; see https://issues.apache.org/jira/browse/ARROW-12201
 fn v1_u32_nullable() -> Result<()> {
     test_pyarrow_integration("uint32", 1, "basic", false, false, None)
 }
 
+#[test]
+fn v2_u32_nullable() -> Result<()> {
+    test_pyarrow_integration("uint32", 2, "basic", false, false, None)
+}
+
 #[test]
 fn v2_int64_nullable() -> Result<()> {
     test_pyarrow_integration("int64", 2, "basic", false, false, None)