Skip to content
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

sql: use the new EXPLAIN infrastructure for UI plans #52956

Merged
merged 3 commits into from
Aug 18, 2020

Conversation

RaduBerinde
Copy link
Member

sql: don't key statement statistics on distribution/vectorization

The statement statistics code buckets statements separately depending on the
distribution and vectorization flags. This is not very useful: we normally don't
expect these flags to change for the same statement; and the UI doesn't show
them separately anyway. In addition, it poses a problem to switching the explain
tree (presented to UI) to use the new explain infrastructure: we need to know if
we will need the tree upfront (before execbuild).

This change removes these flags from the key. The flags are still populated in
the StatementStatisticsKey.

Release note: None

sql: use the new EXPLAIN infrastructure for UI plans

This change reworks how the roachpb.ExplainPlanTreeNode tree (used by the UI
to show statement plans) is populated. We use the new EXPLAIN infrastructure.
The main change is that we need to decide upfront if we will need the plan or
not.

Release note (admin ui change): various improvements to the statement plans in
the UI.

The statement statistics code buckets statements separately depending on the
distribution and vectorization flags. This is not very useful: we normally don't
expect these flags to change for the same statement; and the UI doesn't show
them separately anyway. In addition, it poses a problem to switching the explain
tree (presented to UI) to use the new explain infrastructure: we need to know if
we will need the tree upfront (before execbuild).

This change removes these flags from the key. The flags are still populated in
the `StatementStatisticsKey`.

Release note: None
@RaduBerinde RaduBerinde requested a review from a team as a code owner August 18, 2020 01:49
@cockroach-teamcity
Copy link
Member

This change is Reviewable

To support executing plans that were generated with `explain.Factory`, we must
not use the same factory for "inner" plans generated by apply joins and
recursive CTEs. We modify the functions to pass the exec factory. Note that we
were already doing this for cascade planning.

Release note: None
This change reworks how the `roachpb.ExplainPlanTreeNode` tree (used by the UI
to show statement plans) is populated. We use the new EXPLAIN infrastructure.
The main change is that we need to decide upfront if we will need the plan or
not.

Release note (admin ui change): various improvements to the statement plans in
the UI.
@RaduBerinde
Copy link
Member Author

TFTR!

@RaduBerinde
Copy link
Member Author

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 18, 2020

Build succeeded:

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

Successfully merging this pull request may close these issues.

3 participants