Skip to content

Comments

Change default EXPLAIN format in datafusion-cli to tree format#15427

Merged
alamb merged 10 commits intoapache:mainfrom
alamb:alamb/default_explain_plan
Mar 31, 2025
Merged

Change default EXPLAIN format in datafusion-cli to tree format#15427
alamb merged 10 commits intoapache:mainfrom
alamb:alamb/default_explain_plan

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Mar 25, 2025 •
edited
Loading

Which issue does this PR close?

Rationale for this change

We have nice explain plans, let's default DataFusion cli to use them

What changes are included in this PR?

Note that as discussed on #15343, this ONLY affects datafusion-cli. Any downstream project that wants nicer explain plans by default can adjust the configuration settings

Are these changes tested?

Yes with new tests

Are there any user-facing changes?

EXPLAIN in datafusion-cli will now use the new explain tree format. For example:

DataFusion CLI v46.0.1
> explain select 123;
+---------------+-------------------------------+
| plan_type     | plan                          |
+---------------+-------------------------------+
| physical_plan | ┌───────────────────────────┐ |
|               | │       ProjectionExec      │ |
|               | │    --------------------   │ |
|               | │      Int64(123): 123      │ |
|               | └─────────────┬─────────────┘ |
|               | ┌─────────────┴─────────────┐ |
|               | │     PlaceholderRowExec    │ |
|               | └───────────────────────────┘ |
|               |                               |
+---------------+-------------------------------+
1 row(s) fetched.
Elapsed 0.026 seconds.

You can get the previous indent explain plan by asking for it explicitly:

DataFusion CLI v46.0.1
> explain format indent select 123;
+---------------+------------------------------------------+
| plan_type     | plan                                     |
+---------------+------------------------------------------+
| logical_plan  | Projection: Int64(123)                   |
|               |   EmptyRelation                          |
| physical_plan | ProjectionExec: expr=[123 as Int64(123)] |
|               |   PlaceholderRowExec                     |
|               |                                          |
+---------------+------------------------------------------+
2 row(s) fetched.
Elapsed 0.023 seconds.

alamb
alamb commented Mar 25, 2025
View reviewed changes
datafusion-cli/tests/cli_integration.rs Outdated
assert_cmd_snapshot!(cmd);
}

#[rstest]
Copy link
Contributor Author

@alamb alamb Mar 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could not get these to work as additional examples in quick_test for reasons I couldn't figure out. So I just made a new test

Copy link
Collaborator

@blaginin blaginin Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably because of this

https://github.com/apache/datafusion/pull/15427/files#diff-2382ee3b9da4ce9186e2e8abc1bc3e923f2d3d548de8039dc1f2aa7a21b88942R84

Copy link
Collaborator

@blaginin blaginin Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this works

blaginin@4738e46

Copy link
Contributor Author

@alamb alamb Mar 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, pulled into this branch

@alamb alamb force-pushed the alamb/default_explain_plan branch from c7ced76 to cfa9171 Compare March 25, 2025 22:41
@alamb alamb added the api change Changes the API exposed to users of the crate label Mar 25, 2025
@alamb alamb changed the title Alamb/default explain plan Change default EXPLAIN format in datafusion-cli to tree format Mar 25, 2025
@alamb alamb marked this pull request as ready for review March 26, 2025 10:00
@alamb alamb mentioned this pull request Mar 26, 2025
Closed
xudong963
xudong963 approved these changes Mar 26, 2025
View reviewed changes
Copy link
Member

@xudong963 xudong963 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking forward to df47 😆

@blaginin
Copy link
Collaborator

blaginin commented Mar 26, 2025

I think one issue with the current approach is that loading from env will break.

before:
RustRover 2025-03-26 18 56 08

after:
RustRover 2025-03-26 18 55 58

@alamb
Copy link
Contributor Author

alamb commented Mar 26, 2025

I think one issue with the current approach is that loading from env will break.

this is a good call -- I will fix that

@alamb
Copy link
Contributor Author

alamb commented Mar 27, 2025

Run extended tests

@alamb
Copy link
Contributor Author

alamb commented Mar 27, 2025

Run extended tests

@alamb
Copy link
Contributor Author

alamb commented Mar 30, 2025

I think one issue with the current approach is that loading from env will break.

I pushed a fix and a test for this

blaginin
blaginin approved these changes Mar 30, 2025
View reviewed changes
Copy link
Collaborator

@blaginin blaginin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍

@alamb
Copy link
Contributor Author

alamb commented Mar 30, 2025

I'll plan to merge this one tomorrow unless anyone else would like time to comment

@alamb
Copy link
Contributor Author

alamb commented Mar 31, 2025 •
edited
Loading

🚀 📖

THanks again @blaginin and @xudong963

@alamb alamb merged commit 7850cef into apache:main Mar 31, 2025
27 checks passed
@alamb alamb deleted the alamb/default_explain_plan branch March 31, 2025 14:09
@alamb alamb mentioned this pull request Apr 1, 2025
Closed
This was referenced Apr 14, 2025
Closed
Merged
nirnayroy pushed a commit to nirnayroy/datafusion that referenced this pull request May 2, 2025
@alamb @blaginin @cipherstakes
Change default EXPLAIN format in datafusion-cli to tree format (a…
955b98c 
…pache#15427)

* Change default explain plan to `tree`

* Add tests

* remove unecessary changes

* more unecessary changes

* Move test cases

* Respect environment variable for explain plan

* Fix format

* fmt

---------

Co-authored-by: blaginin <[email protected]>
@petern48 petern48 mentioned this pull request Sep 9, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@blaginin blaginin blaginin approved these changes

@xudong963 xudong963 xudong963 approved these changes

Assignees

No one assigned

Labels

api change Changes the API exposed to users of the crate

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[DISCUSS] Switch to tree explain by default

3 participants

@alamb @blaginin @xudong963