-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Unparse Sort with pushdown limit to SQL string #12873
Unparse Sort with pushdown limit to SQL string #12873
Conversation
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.
lgtm, but see question
datafusion/sql/src/unparser/plan.rs
Outdated
input: Arc::clone(&sort.input), | ||
fetch: None, | ||
})); | ||
let limit = Arc::new(LogicalPlan::Limit(Limit { |
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.
Is it possible to unparse without creating intermediate (fake) plan nodes?
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.
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.
thanks!
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.
Thanks @goldmedal and @findepi
* unparse Sort with push down limit * cargo fmt * set query limit directly
* unparse Sort with push down limit * cargo fmt * set query limit directly
* init (apache#12453) * Fix unparse table scan with the projection pushdown (apache#12534) * unparse the projection base on the source schema * refactor and enhance the test * Fix unparsing OFFSET (apache#12539) * Unparse Sort with pushdown limit to SQL string (apache#12873) * unparse Sort with push down limit * cargo fmt * set query limit directly * Unparse `SubqueryAlias` without projections to SQL (apache#12896) * change pub function comment to doc * unparse subquery alias without projections * fix tests * rollback the empty line * rollback the empty line * exclude the table_scan with pushdown case * fmt and clippy * simplify the ast to string and remove unused debug code * enhance unparsing plan with pushdown to avoid unnamed subquery (apache#13006) * Update --------- Co-authored-by: Lordworms <[email protected]> Co-authored-by: Jax Liu <[email protected]> Co-authored-by: Justus Flerlage <[email protected]>
Which issue does this PR close?
no corresponding issue.
Rationale for this change
After the plan optimization, the limit could be pushed down to Sort. Unparser can't handle the pushdown limit currently.
Consider the following plan:
It should be unparsed to
What changes are included in this PR?
Are these changes tested?
yes
Are there any user-facing changes?
no