yonatan-sevenai commented on code in PR #21099:
URL: https://github.com/apache/datafusion/pull/21099#discussion_r3034969487
##########
datafusion/sql/src/unparser/plan.rs:
##########
@@ -828,6 +828,27 @@ impl Unparser<'_> {
Some(plan_alias.alias.clone()),
select.already_projected(),
)?;
+
+ // If the SubqueryAlias directly wraps a plan that builds its
+ // own SELECT clauses (e.g. Aggregate adds GROUP BY, Window
adds
+ // OVER, etc.) and unparse_table_scan_pushdown couldn't reduce
it,
+ // we must emit a derived subquery: (SELECT ...) AS alias.
+ // Without this, the recursive handler would merge those
clauses
+ // into the outer SELECT, losing the subquery structure
entirely.
+ if unparsed_table_scan.is_none()
+ &&
Self::requires_derived_subquery(plan_alias.input.as_ref())
Review Comment:
Good catch — updated the check and the derive call to use the rewritten plan
instead of plan_alias.input. This also needed a small addition: when the derive
path is taken with column
aliases on a dialect that doesn't support them in table aliases (e.g.
SQLite), we now inject them into the inner projection first, same as the
non-derive path already does.
##########
datafusion/sql/src/unparser/plan.rs:
##########
@@ -828,6 +828,27 @@ impl Unparser<'_> {
Some(plan_alias.alias.clone()),
select.already_projected(),
)?;
+
+ // If the SubqueryAlias directly wraps a plan that builds its
+ // own SELECT clauses (e.g. Aggregate adds GROUP BY, Window
adds
+ // OVER, etc.) and unparse_table_scan_pushdown couldn't reduce
it,
+ // we must emit a derived subquery: (SELECT ...) AS alias.
+ // Without this, the recursive handler would merge those
clauses
+ // into the outer SELECT, losing the subquery structure
entirely.
+ if unparsed_table_scan.is_none()
+ &&
Self::requires_derived_subquery(plan_alias.input.as_ref())
Review Comment:
Good catch - updated the check and the derive call to use the rewritten plan
instead of plan_alias.input. This also needed a small addition: when the derive
path is taken with column
aliases on a dialect that doesn't support them in table aliases (e.g.
SQLite), we now inject them into the inner projection first, same as the
non-derive path already does.
--
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]
