Skip to content

Implement tree explain for AggregateExec #15024

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

Closed
Tracked by #14914
alamb opened this issue Mar 5, 2025 · 4 comments · Fixed by #15103
Closed
Tracked by #14914

Implement tree explain for AggregateExec #15024

alamb opened this issue Mar 5, 2025 · 4 comments · Fixed by #15103
Assignees
Labels
good first issue Good for newcomers

Comments

@alamb
Copy link
Contributor

alamb commented Mar 5, 2025

Is your feature request related to a problem or challenge?

@irenjj added a new tree explain mode in #14677. Now we need to add support for different types of operators.

Here is an example of how to see the new explain plans:

set datafusion.explain.format = 'tree';
create table foo(x int, y int) as values (1,2), (3,4);
explain select * from foo where x = 4;
+---------------+------------------------------------+
| plan_type     | plan                               |
+---------------+------------------------------------+
| logical_plan  | Filter: foo.x = Int32(4)           |
|               |   TableScan: foo projection=[x, y] |
| physical_plan | ┌───────────────────────────┐      |
|               | │    CoalesceBatchesExec    │      |
|               | └─────────────┬─────────────┘      |
|               | ┌─────────────┴─────────────┐      |
|               | │         FilterExec        │      |
|               | └─────────────┬─────────────┘      |
|               | ┌─────────────┴─────────────┐      |
|               | │       DataSourceExec      │      |
|               | │    --------------------   │      |
|               | │    partition_sizes: [1]   │      |
|               | │       partitions: 1       │      |
|               | └───────────────────────────┘      |
|               |                                    |
+---------------+------------------------------------+

Describe the solution you'd like

Add tree format to the ExecutionPlan specified in the subject of this ticket

Note that the tree mode should have only the most relevant information for users to understand what plan is being run. Detailed iformation

The process goes like:

  1. Add the relevant code to the operator
  2. Add / update explain_tree.slt test to show the new code in action

The relevant code looks like this

            DisplayFormatType::TreeRender => {
                // TODO: collect info
                write!(f, "")?;
            }

You can run the tests like

cargo test --test sqllogictests -- explain_tree

You can update the test like this:

cargo test --test sqllogictests -- explain_tree --complete

Describe alternatives you've considered

Here is an example PR that shows what is needed

Additional context

Since this is well specified and has several examples and will give exposure to DataFusion code and testing, I have marked this as a good first issue

@alamb alamb added the good first issue Good for newcomers label Mar 5, 2025
@zebsme
Copy link
Contributor

zebsme commented Mar 5, 2025

take

@alamb
Copy link
Contributor Author

alamb commented Mar 7, 2025

@zebsme how is this going? I noticed you took this and #15025 and #15026

If you aren't planning on working on them in the near term perhaps you could mention that on the tickets so others who might have time can work on them

@zebsme
Copy link
Contributor

zebsme commented Mar 8, 2025

@zebsme how is this going? I noticed you took this and #15025 and #15026

If you aren't planning on working on them in the near term perhaps you could mention that on the tickets so others who might have time can work on them

hi,I only took this,15025 and 15026 are not my tickets. And I will work on this tomorrow

@alamb
Copy link
Contributor Author

alamb commented Mar 8, 2025

@zebsme how is this going? I noticed you took this and #15025 and #15026
If you aren't planning on working on them in the near term perhaps you could mention that on the tickets so others who might have time can work on them

hi,I only took this,15025 and 15026 are not my tickets. And I will work on this tomorrow

Sorry about that -- yes I confused your github handle and @zjregee -- I apologize 🤦

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants