forked from apache/datafusion
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Matt/recursive cte eliminate distribution and coalesce in recursive children #2
Draft
matthewgapp
wants to merge
15
commits into
matt/recursive-cte
Choose a base branch
from
matt/recursive-cte-eliminate-distribution-and-coalesce-in-recursive-children
base: matt/recursive-cte
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
matthewgapp
force-pushed
the
matt/recursive-cte
branch
from
November 18, 2023 21:44
f5d7eba
to
e8d78c8
Compare
matthewgapp
force-pushed
the
matt/recursive-cte-eliminate-distribution-and-coalesce-in-recursive-children
branch
from
November 18, 2023 22:08
81c3439
to
b7e59f7
Compare
rebased onto updated (rebased) cte PR. Plan to ready this PR by tomorrow. |
matthewgapp
force-pushed
the
matt/recursive-cte-eliminate-distribution-and-coalesce-in-recursive-children
branch
from
November 19, 2023 01:12
2461576
to
ce3cdcd
Compare
matthewgapp
force-pushed
the
matt/recursive-cte
branch
2 times, most recently
from
January 9, 2024 06:27
7dba5b6
to
219de0c
Compare
Repository owner
deleted a comment from
tobarbaro
Feb 10, 2024
matthewgapp
pushed a commit
that referenced
this pull request
Apr 12, 2024
* refactor `TreeNode::rewrite()` * use handle_tree_recursion in `Expr` * use macro for transform recursions * fix api * minor fixes * fix * don't trust `t.transformed` coming from transformation closures, keep the old way of detecting if changes were made * rephrase todo comment, always propagate up `t.transformed` from the transformation closure, fix projection pushdown closure * Fix `TreeNodeRecursion` docs * extend Skip (Prune) functionality to Jump as it is defined in https://synnada.notion.site/synnada/TreeNode-Design-Proposal-bceac27d18504a2085145550e267c4c1 * fix Jump and add tests * jump test fixes * fix clippy * unify "transform" traversals using macros, fix "visit" traversal jumps, add visit jump tests, ensure consistent naming `f` instead of `op`, `f_down` instead of `pre_visit` and `f_up` instead of `post_visit` * fix macro rewrite * minor fixes * minor fix * refactor tests * add transform tests * add apply, transform_down and transform_up tests * refactor tests * test jump on both a and e nodes in both top-down and bottom-up traversals * better transform/rewrite tests * minor fix * simplify tests * add stop tests, reorganize tests * fix previous merges and remove leftover file * Review TreeNode Refactor (#1) * Minor changes * Jump doesn't ignore f_up * update test * Update rewriter * LogicalPlan visit update and propagate from children flags * Update tree_node.rs * Update map_children's --------- Co-authored-by: Mustafa Akur <mustafa.akur@synnada.ai> * fix * minor fixes * fix f_up call when f_down returns jump * simplify code * minor fix * revert unnecessary changes * fix `DynTreeNode` and `ConcreteTreeNode` `transformed` and `tnr` propagation * introduce TransformedResult helper * fix docs * restore transform as alias to trassform_up * restore transform as alias to trassform_up 2 * Simplifications and comment improvements (#2) --------- Co-authored-by: Berkay Şahin <124376117+berkaysynnada@users.noreply.github.com> Co-authored-by: Mustafa Akur <mustafa.akur@synnada.ai> Co-authored-by: Mehmet Ozan Kabak <ozankabak@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This a WIP. It attempts to eliminate unnecessary (and costly) physical plan steps within a recursive query, speeding recursive queries up by ~30x but perhaps more in some cases.
Specifically, it prevents RepartitionExec and CoalesceExec when they'd otherwise be decedents of a RecursiveQueryExec. It does, however, allow them to be applied to parts of the plan that aren't under a RecursiveQueryExec.
The PR makes changes to DistributionContext and introduces the context pattern into the Coalesce physical optimizer to determine if a given plan is within a RecursiveQuery.
Example:
Before
cargo run -r
time: 6.5 secondsAfter
cargo run -r
time: 220msWhich issue does this PR close?
Closes #.
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?