Display: Support preserve_partitioning on SortExec physical plan.
#10153
Conversation
|
I think you can update the tests using completion mode: https://github.com/apache/datafusion/tree/main/datafusion/sqllogictest#updating-tests-completion-mode That probably will get this PR mostly working |
Fixes: apache#10138 `Display` of SortExec execution plan now support `preserve_partitioning` field Signed-off-by: Kaviraj <[email protected]>
Signed-off-by: Kaviraj <[email protected]>
kavirajk
force-pushed
the
kavirajk/sortexec-physical-plan-format
branch
from
56b18c7
to
6dbe69a
Compare
Signed-off-by: Kaviraj <[email protected]>
|
@alamb thanks for the suggestion 👍 . Quick follow up. That command you mentioned I don't see it updates any of the other test cases where we are asserting the physical plan string representation. That still needs manual updates correct (around 79 tests were failing)?. I started updating those, just wanted to check if there are any easy/automated way that I'm not aware of :) |
Signed-off-by: Kaviraj <[email protected]>
Signed-off-by: Kaviraj <[email protected]>
Signed-off-by: Kaviraj <[email protected]>
Signed-off-by: Kaviraj <[email protected]>
| @@ -868,11 +868,12 @@ impl DisplayAs for SortExec { | |||
| match t { | |||
| DisplayFormatType::Default | DisplayFormatType::Verbose => { | |||
| let expr = PhysicalSortExpr::format_list(&self.expr); | |||
| let preserve_partioning = self.preserve_partitioning; | |||
There was a problem hiding this comment.
NOTE for reviewers: These changes in sort.rs is the only real change for this PR.
Everything else is just update tests either manually or automatically (see PR description for steps I used to update those)
…sical-plan-format
|
I took the liberty of merging up from main to this branch to ensure we didn't have any logical conflicts (e.g. newly added explain plans for example). I think we can merge it once CI passes |
|
Thanks again @kavirajk |
Which issue does this PR close?
Closes #10138
Rationale for this change
Displayof SortExec execution plan doesn't have visibility onpreserve_partitioningfieldWhat changes are included in this PR?
Only important change made in this PR is
Displayof SortExec execution plan now supportpreserve_partitioningfieldEverything else is either updating tests manually or via some script. Following are the steps I used to update tests automatically (thanks @alamb for the tip)
cargo test --test sqllogictests -- --complete- that updates all.sltfiles except ones fromtpchbenchmarkstpchbenchmarks data using steps mentioned here.INCLUDE_TPCH=true cargo test --test sqllogictests -- --complete- to updatetpchbenchmarks tests outputs.Are these changes tested?
Yes
Are there any user-facing changes?
May be, if user try to format the Sort execution plan.