[GitHub] [arrow-rs] alamb commented on a change in pull request #1149: feat(json): support for map arrays in json writer

[GitBox <gi...@apache.org>]

alamb commented on a change in pull request #1149:
URL: https://github.com/apache/arrow-rs/pull/1149#discussion_r782430643



##########
File path: arrow/src/json/writer.rs
##########
@@ -486,6 +486,41 @@ fn set_column_for_json_rows(
                 .expect("cannot cast dictionary to underlying values");
             set_column_for_json_rows(rows, row_count, &hydrated, col_name)
         }
+        DataType::Map(_, _) => {
+            let maparr = as_map_array(array);
+
+            let keys = maparr.keys();
+            let values = maparr.values();
+
+            // Keys have to be strings to convert to json.
+            if !matches!(keys.data_type(), DataType::Utf8) {
+                panic!("Unsupported datatype: {:#?}", array.data_type());

Review comment:
       it would be much nicer if this code did not `panic` when there was an unsupported datatype. However, I see that the the signature for `set_column_for_json_rows` does not return a Result 😢 
   
   I'll file a follow on ticket and see if someone in the community is interesting in improving this

##########
File path: arrow/src/json/writer.rs
##########
@@ -1312,6 +1347,65 @@ mod tests {
 {}
 {"list":[{}]}
 {"list":[{}]}
+"#
+        );
+    }
+
+    #[test]
+    fn json_writer_map() {
+        let keys_array =
+            super::StringArray::from(vec!["foo", "bar", "baz", "qux", "quux"]);
+        let values_array = super::Int64Array::from(vec![10, 20, 30, 40, 50]);
+
+        let keys = Field::new("keys", DataType::Utf8, false);
+        let values = Field::new("values", DataType::Int64, false);
+        let entry_struct = StructArray::from(vec![
+            (keys, Arc::new(keys_array) as ArrayRef),
+            (values, Arc::new(values_array) as ArrayRef),
+        ]);
+
+        let map_data_type = DataType::Map(
+            Box::new(Field::new(
+                "entries",
+                entry_struct.data_type().clone(),
+                true,
+            )),
+            false,
+        );
+
+        // [{"foo": 10}, null, {}, {"bar": 20, "baz": 30, "qux": 40}, {"quux": 50}, {}]

Review comment:
       It would be great to have a better / nicer way to contruct `MapArrays` -- again not for this PR but eventually it would be nice to have something like
   
   https://docs.rs/arrow/6.5.0/arrow/array/struct.StructArray.html
   
   I will file another ticket




-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org