sgrebnov commented on code in PR #13880:
URL: https://github.com/apache/datafusion/pull/13880#discussion_r1895049631
##########
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:
@goldmedal - I'm not sure using the last unparsing result is the expected
behavior. 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.
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? To match `unparse` behavior where we don't
know/track if unparsing is applied so we always apply all?
--
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
