goldmedal commented on code in PR #13880:
URL: https://github.com/apache/datafusion/pull/13880#discussion_r1895362378
##########
datafusion/sql/src/unparser/mod.rs:
##########
@@ -105,13 +109,33 @@ impl<'a> Unparser<'a> {
self.pretty = pretty;
self
}
+
+ /// Add a custom unparser for user defined logical nodes
+ ///
+ /// DataFusion allows user to define custom logical nodes. This method
allows to add custom child unparsers for these nodes.
+ /// Implementation of [`UserDefinedLogicalNodeUnparser`] can be added to
the root unparser to handle custom logical nodes.
+ ///
+ /// The child unparsers are called iteratively.
+ /// There are two methods in [`Unparser`] will be called:
+ /// - `extension_to_statement`: This method is called when the custom
logical node is a custom statement.
+ /// If multiple child unparsers return a non-None value, the last
unparsing result will be returned.
Review Comment:
> Is there a specific use case / reason for using the last supported
udlp_unparser? They can be dynamically registered and the last one should
override perviously registered.
>
Actually, I don't have a real case for it but yes, I imagined the user can
append the new unparser to override the previous.
However, I guess it could be a rare use case (?
> As a user, I would expect to get the result from the first udlp_unparser
that supports this node and stop checking the remaining udlp_unparsers instead.
>
Maybe you guys are right. We should make the first one win. It's more
efficient and simpler.
It's also how `ExprPlanner` worked in the planner.
https://github.com/apache/datafusion/blob/75202b587108cd8de6c702a84463ac0ca0ca4d4b/datafusion/sql/src/expr/mod.rs#L222-L229
Anyway, I'll change it. Thanks!
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
