goldmedal commented on code in PR #13824:
URL: https://github.com/apache/datafusion/pull/13824#discussion_r1894580268
##########
datafusion/sql/src/unparser/plan.rs:
##########
@@ -723,19 +733,48 @@ impl Unparser<'_> {
internal_err!("Unnest input is not a Projection:
{unnest:?}")
}
}
- _ => not_impl_err!("Unsupported operator: {plan:?}"),
+ LogicalPlan::Subquery(subquery)
+ if find_unnest_node_until_relation(subquery.subquery.as_ref())
+ .is_some() =>
+ {
+ if self.dialect.unnest_as_table_factor() {
+ self.select_to_sql_recursively(
+ subquery.subquery.as_ref(),
+ query,
+ select,
+ relation,
+ )
+ } else {
+ self.derive_with_dialect_alias(
+ "derived_unnest",
+ subquery.subquery.as_ref(),
+ relation,
+ true,
+ )
+ }
+ }
+ _ => {
+ not_impl_err!("Unsupported operator: {plan:?}")
+ }
}
}
/// Try to find the placeholder column name generated by
`RecursiveUnnestRewriter`
- /// Only match the pattern
`Expr::Alias(Expr::Column("__unnest_placeholder(...)"))`
- fn is_unnest_placeholder(expr: &Expr) -> bool {
+ /// The first return value is a boolean indicating if the column is a
placeholder column:
+ /// Try to match the pattern
`Expr::Alias(Expr::Column("__unnest_placeholder(...)"))`
+ /// The second return value is a boolean indicating if the column uses an
outer reference:
+ /// Try to match the pattern
`Expr::Alias(Expr::Column("__unnest_placeholder(outer_ref(...)))")`
+ ///
+ /// `outer_ref` is the display result of [Expr::OuterReferenceColumn]
+ fn is_unnest_placeholder_with_outer_ref(expr: &Expr) -> (bool, bool) {
if let Expr::Alias(Alias { expr, .. }) = expr {
if let Expr::Column(Column { name, .. }) = expr.as_ref() {
- return name.starts_with(UNNEST_PLACEHOLDER);
+ if let Some(prefix) = name.strip_prefix(UNNEST_PLACEHOLDER) {
+ return (true, prefix.starts_with("(outer_ref("));
Review Comment:
> Not a huge fan of this string matching.
I agree that string matching isn't a good way but I think it's the only way
to recognize if the outer reference column is exits in the unnest plan
currently. 🤔
> At least the UNNEST_PLACEHOLDER is a shared const, but the `outer_ref`
could potentially change and we would miss it here.
Maybe we can use something like `UNNEST_PLACEHOLDER` to create a global
constant for `outer_ref`. At least then, we won't miss it when someone needs to
change it.
https://github.com/apache/datafusion/blob/ede665b212ec5e3a673986424103645bcb49e3bc/datafusion/expr/src/expr.rs#L2546
--
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
