Blizzara commented on code in PR #16345:
URL: https://github.com/apache/datafusion/pull/16345#discussion_r2140656890


##########
datafusion/substrait/src/logical_plan/consumer/utils.rs:
##########
@@ -81,98 +81,167 @@ pub(super) fn next_struct_field_name(
     }
 }
 
-pub(super) fn rename_field(
+/// Traverse through the field, renaming the provided field itself and all its 
inner struct fields.
+pub fn rename_field(

Review Comment:
   I think 1 and 2 are a bit internally conflicting, right? 😄 As in, (2) 
answers to the question of (1). These are very specific to Substrait handling - 
I don't know any other places where one would one to rename 
schema/fields/datatypes this way, but it does come up when dealing with 
Substrait.
   
   Given that, I don't know of a usecase that'd benefit from (2), and I think 
it's reasonable for these to be in (1). Does that make sense? :)
   
   And as reply to @westonpace as well, I would like for these to be public 
since we do need to deal with Substrait name(less)s also in our own code, 
either when implementing custom Substrait handling rules or for some UDFs. It's 
not a big deal to copy these either, but I'd prefer not to 😄 



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