jonahgao commented on code in PR #10531:
URL: https://github.com/apache/datafusion/pull/10531#discussion_r1614666495


##########
datafusion/substrait/src/logical_plan/producer.rs:
##########
@@ -512,6 +572,62 @@ pub fn to_substrait_rel(
     }
 }
 
+fn to_substrait_named_struct(schema: &DFSchemaRef) -> Result<NamedStruct> {
+    // Substrait wants a list of all field names, including nested fields from 
structs,
+    // also from within lists and maps. However, it does not want the list and 
map field names
+    // themselves - only structs are considered to have useful names.
+    fn names_dfs(dtype: &DataType) -> Result<Vec<String>> {
+        match dtype {
+            DataType::Struct(fields) => {
+                let mut names = Vec::new();
+                for field in fields {
+                    names.push(field.name().to_string());
+                    names.extend(names_dfs(field.data_type())?);
+                }
+                Ok(names)
+            }
+            DataType::List(l) => names_dfs(l.data_type()),

Review Comment:
   I think we also need to consider `DataType::LargeList` here because 
`to_substrait_type` supports it.



-- 
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-unsubscr...@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to