alamb commented on a change in pull request #1091:
URL: https://github.com/apache/arrow-datafusion/pull/1091#discussion_r725621889



##########
File path: datafusion/src/scalar.rs
##########
@@ -1901,5 +2037,191 @@ mod tests {
             )),
             None
         );
+

Review comment:
       These are very nice and thorough tests.
   
   The only thing I didn't see a test for was struct recursion -- like
   
   ```
   A : {
     B: "foo"
     C: "bar"
   }
   ```
   
   I wonder if you could extend the `A`, `B`, `C` example below to include a 
nested struct as well?

##########
File path: datafusion/src/scalar.rs
##########
@@ -1171,6 +1277,20 @@ impl FromStr for ScalarValue {
     }
 }
 
+impl From<Vec<(&str, ScalarValue)>> for ScalarValue {
+    fn from(value: Vec<(&str, ScalarValue)>) -> Self {
+        let mut fields: Vec<Field> = Vec::new();
+        let mut scalars: Vec<ScalarValue> = Vec::new();
+
+        value.iter().for_each(|(name, scalar)| {
+            fields.push(Field::new(name, scalar.get_datatype(), false));
+            scalars.push(scalar.clone());
+        });

Review comment:
       You might be able to write this in a more functional style (and avoid 
`mut`) using `unzip`: 
https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.unzip
   
   Something like (untested)
   
   ```
   let (fields, scalars) = value.iter()
         .for_each(|(name, scalar)| {
               (Field::new(name, scalar.get_datatype(), false),
               scalar.clone())
           })
         .unzip();
   ```




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to