You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "sgrebnov (via GitHub)" <gi...@apache.org> on 2024/11/29 06:39:51 UTC
[PR] Improve unparsing after optimize_projections optimization [datafusion]
sgrebnov opened a new pull request, #13599:
URL: https://github.com/apache/datafusion/pull/13599
## Which issue does this PR close?
Follow-up item for [Support unparsing plans after applying the `optimize_projections` rule PR](https://github.com/apache/datafusion/pull/13267), focusing on the unparsing enhancement to support the described scenario.
With the `optimize_projections` optimization, columns are pushed down to the `TableScan`, resulting in both `Projection` and `TableScan` nodes containing projection information. As a result, the generated SQL contains a duplicate subquery, e.g., `SELECT a, b FROM (SELECT a, b FROM my_table).`
## What changes are included in this PR?
PR updates `unparse_table_scan_pushdown` to prevent the creation of an additional projection if the unparser already includes a projection (`SELECT`) part.
## Are these changes tested?
Yes, TPC-H, TPC-DS benchmark tests, unit tests.
## Are there any user-facing changes?
Improved Plan-to-SQL conversion when a TableScan with projection information is wrapped in a Projection.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve unparsing after optimize_projections optimization [datafusion]
Posted by "goldmedal (via GitHub)" <gi...@apache.org>.
goldmedal commented on PR #13599:
URL: https://github.com/apache/datafusion/pull/13599#issuecomment-2516157511
Thanks @sgrebnov and @alamb @phillipleblanc for reviewing
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve unparsing after optimize_projections optimization [datafusion]
Posted by "goldmedal (via GitHub)" <gi...@apache.org>.
goldmedal merged PR #13599:
URL: https://github.com/apache/datafusion/pull/13599
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve unparsing after optimize_projections optimization [datafusion]
Posted by "sgrebnov (via GitHub)" <gi...@apache.org>.
sgrebnov commented on code in PR #13599:
URL: https://github.com/apache/datafusion/pull/13599#discussion_r1863025876
##########
datafusion/sql/tests/cases/plan_to_sql.rs:
##########
@@ -926,12 +926,25 @@ fn test_table_scan_pushdown() -> Result<()> {
let query_from_table_scan_with_projection = LogicalPlanBuilder::from(
table_scan(Some("t1"), &schema, Some(vec![0, 1]))?.build()?,
)
- .project(vec![wildcard()])?
Review Comment:
Can this actually be a real plan (other than manually constructed) with a wildcard projection and a TableScan that includes only two columns?
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve unparsing after optimize_projections optimization [datafusion]
Posted by "sgrebnov (via GitHub)" <gi...@apache.org>.
sgrebnov commented on code in PR #13599:
URL: https://github.com/apache/datafusion/pull/13599#discussion_r1866884796
##########
datafusion/sql/tests/cases/plan_to_sql.rs:
##########
@@ -926,12 +926,25 @@ fn test_table_scan_pushdown() -> Result<()> {
let query_from_table_scan_with_projection = LogicalPlanBuilder::from(
table_scan(Some("t1"), &schema, Some(vec![0, 1]))?.build()?,
)
- .project(vec![wildcard()])?
Review Comment:
@goldmedal - thank you for confirming, good to know!
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve unparsing after optimize_projections optimization [datafusion]
Posted by "goldmedal (via GitHub)" <gi...@apache.org>.
goldmedal commented on code in PR #13599:
URL: https://github.com/apache/datafusion/pull/13599#discussion_r1866869400
##########
datafusion/sql/tests/cases/plan_to_sql.rs:
##########
@@ -926,12 +926,25 @@ fn test_table_scan_pushdown() -> Result<()> {
let query_from_table_scan_with_projection = LogicalPlanBuilder::from(
table_scan(Some("t1"), &schema, Some(vec![0, 1]))?.build()?,
)
- .project(vec![wildcard()])?
Review Comment:
It won't be a real case in DataFusion because the wildcard char will be expanded by `ExpandWildcardRule`
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve unparsing after optimize_projections optimization [datafusion]
Posted by "sgrebnov (via GitHub)" <gi...@apache.org>.
sgrebnov commented on code in PR #13599:
URL: https://github.com/apache/datafusion/pull/13599#discussion_r1863025876
##########
datafusion/sql/tests/cases/plan_to_sql.rs:
##########
@@ -926,12 +926,25 @@ fn test_table_scan_pushdown() -> Result<()> {
let query_from_table_scan_with_projection = LogicalPlanBuilder::from(
table_scan(Some("t1"), &schema, Some(vec![0, 1]))?.build()?,
)
- .project(vec![wildcard()])?
Review Comment:
Can this actually be a real plan (other than manually constructed) with a wildcard projection and a TableScan that includes only two columns? I would expect them to match.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve unparsing after optimize_projections optimization [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #13599:
URL: https://github.com/apache/datafusion/pull/13599#issuecomment-2512502563
@phillipleblanc do you potentially have time to review this PR?
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org