-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce UserDefinedLogicalNodeUnparser
for User-defined Logical Plan unparsing
#13880
Changes from 6 commits
c76dbae
2335276
640bc93
4a32991
35dac96
85fb3a4
abc23f0
5233e4a
856a5f5
7b6c37f
b2654df
0bae30a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -17,17 +17,19 @@ | |||||||||||||||||
|
||||||||||||||||||
//! [`Unparser`] for converting `Expr` to SQL text | ||||||||||||||||||
|
||||||||||||||||||
mod ast; | ||||||||||||||||||
pub mod ast; | ||||||||||||||||||
mod expr; | ||||||||||||||||||
mod plan; | ||||||||||||||||||
mod rewrite; | ||||||||||||||||||
mod utils; | ||||||||||||||||||
|
||||||||||||||||||
use self::dialect::{DefaultDialect, Dialect}; | ||||||||||||||||||
use crate::unparser::udlp_unparser::UserDefinedLogicalNodeUnparser; | ||||||||||||||||||
pub use expr::expr_to_sql; | ||||||||||||||||||
pub use plan::plan_to_sql; | ||||||||||||||||||
|
||||||||||||||||||
use self::dialect::{DefaultDialect, Dialect}; | ||||||||||||||||||
use std::sync::Arc; | ||||||||||||||||||
pub mod dialect; | ||||||||||||||||||
pub mod udlp_unparser; | ||||||||||||||||||
|
||||||||||||||||||
/// Convert a DataFusion [`Expr`] to [`sqlparser::ast::Expr`] | ||||||||||||||||||
/// | ||||||||||||||||||
|
@@ -55,13 +57,15 @@ pub mod dialect; | |||||||||||||||||
pub struct Unparser<'a> { | ||||||||||||||||||
dialect: &'a dyn Dialect, | ||||||||||||||||||
pretty: bool, | ||||||||||||||||||
udlp_unparsers: Vec<Arc<dyn UserDefinedLogicalNodeUnparser>>, | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
impl<'a> Unparser<'a> { | ||||||||||||||||||
pub fn new(dialect: &'a dyn Dialect) -> Self { | ||||||||||||||||||
Self { | ||||||||||||||||||
dialect, | ||||||||||||||||||
pretty: false, | ||||||||||||||||||
udlp_unparsers: vec![], | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
|
@@ -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. | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I would also expect this to short-circuit and have the first one win. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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.
Maybe you guys are right. We should make the first one win. It's more efficient and simpler. datafusion/datafusion/sql/src/expr/mod.rs Lines 222 to 229 in 75202b5
Anyway, I'll change it. Thanks! |
||||||||||||||||||
/// - `extension_to_sql`: This method is called when the custom logical node is part of a statement. | ||||||||||||||||||
/// If multiple child unparsers are registered for the same custom logical node, all of them will be called in order. | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should also short-circuit and only do the first one? |
||||||||||||||||||
pub fn with_udlp_unparsers( | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a fan of this name - There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds great. I'll rename it. |
||||||||||||||||||
mut self, | ||||||||||||||||||
udlp_unparsers: Vec<Arc<dyn UserDefinedLogicalNodeUnparser>>, | ||||||||||||||||||
) -> Self { | ||||||||||||||||||
self.udlp_unparsers = udlp_unparsers; | ||||||||||||||||||
self | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
impl Default for Unparser<'_> { | ||||||||||||||||||
fn default() -> Self { | ||||||||||||||||||
Self { | ||||||||||||||||||
dialect: &DefaultDialect {}, | ||||||||||||||||||
pretty: false, | ||||||||||||||||||
udlp_unparsers: vec![], | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is very cool