sql: display postqueries in EXPLAIN #38849
Conversation
|
This change is |
There was a problem hiding this comment.
Thanks for this!
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis, @RaduBerinde, and @yuzefovich)
pkg/sql/logictest/testdata/planner_test/postquery, line 1 at r1 (raw file):
# LogicTest: local-opt fakedist-opt
This is great, except this should live in opt/execbuilder/testdata (everything in planner_test is non-opt). Can you also rename it to fk_opt? To correspond with the fk_opt file in logictests.
pkg/sql/logictest/testdata/planner_test/postquery, line 13 at r1 (raw file):
statement ok INSERT INTO parent VALUES (1), (2)
[nit] remove this (data doesn't matter when we're just explaining)
yuzefovich
force-pushed
the
explain-postqueries
branch
from
13295ae to
42cc321
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/sql/logictest/testdata/planner_test/postquery, line 1 at r1 (raw file):
Previously, RaduBerinde wrote…
This is great, except this should live in
opt/execbuilder/testdata(everything in planner_test is non-opt). Can you also rename it tofk_opt? To correspond with thefk_optfile in logictests.
Done.
pkg/sql/logictest/testdata/planner_test/postquery, line 13 at r1 (raw file):
Previously, RaduBerinde wrote…
[nit] remove this (data doesn't matter when we're just explaining)
Indeed, done.
There was a problem hiding this comment.
Reviewable status:
pkg/sql/explain_plan.go, line 368 at r2 (raw file):
return err } observer.attr("postquery", "id", fmt.Sprintf("@S%d", i+1))
[nit] I don't think this id is useful in the output. I will (after this PR merges) add a field to the node with a "description" of the postquery, (e.g. FK-check-<fk-name>).
pkg/sql/opt/exec/execbuilder/testdata/fk_opt, line 1 at r2 (raw file):
# LogicTest: local-opt fakedist-opt
remove fakedist-opt (local vs fakedist makes no difference in these tests where we just EXPLAIN)
There was a problem hiding this comment.
Reviewable status:
pkg/sql/explain_plan.go, line 368 at r2 (raw file):
Previously, RaduBerinde wrote…
[nit] I don't think this id is useful in the output. I will (after this PR merges) add a field to the node with a "description" of the postquery, (e.g.
FK-check-<fk-name>).
Yeah, I was also thinking it's not useful. Removed.
pkg/sql/opt/exec/execbuilder/testdata/fk_opt, line 1 at r2 (raw file):
Previously, RaduBerinde wrote…
remove fakedist-opt (local vs fakedist makes no difference in these tests where we just EXPLAIN)
Done.
yuzefovich
force-pushed
the
explain-postqueries
branch
from
42cc321 to
89f261b
Compare
There was a problem hiding this comment.
Reviewable status:
Postqueries are now displayed in the output of EXPLAIN statement. Release note: None
yuzefovich
force-pushed
the
explain-postqueries
branch
from
89f261b to
19bc3ac
Compare
|
TFTR! bors r+ |
Build failed |
|
Seems like a flake. bors r+ |
38849: sql: display postqueries in EXPLAIN r=yuzefovich a=yuzefovich Postqueries are now displayed in the output of EXPLAIN statement. Fixes: #38340. Release note: None Co-authored-by: Yahor Yuzefovich <[email protected]>
Build succeeded |
Postqueries are now displayed in the output of EXPLAIN statement.
Fixes: #38340.
Release note: None