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

Add an EXPLAIN option to show the Equivalences Analysis #30690

Merged
merged 3 commits into from
Dec 3, 2024

Conversation

ggevay
Copy link
Contributor

@ggevay ggevay commented Dec 2, 2024

We have a bunch of EXPLAIN options to show the results of various Analyses. These are quite useful for debugging. This PR adds an EXPLAIN option for the Equivalences Analysis. This makes the behavior of the Equivalences Analysis easier to understand, especially for people who are not already familiar with all its details.

Motivation

Tips for reviewer

The first 2 commits just do very minor refactorings. The main thing is in the 3rd commit.

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

The Attribute framework evolved into the Analysis framework. However,
the EXPLAIN code still had a bunch of things whose names referred to
Attributes. This commit just renames these things and fixes some
comments.
@ggevay ggevay added A-optimization Area: query optimization and transformation A-CLUSTER Topics related to the CLUSTER layer labels Dec 2, 2024
@ggevay ggevay force-pushed the explain-equivalences branch 6 times, most recently from 6139187 to 7ee105b Compare December 2, 2024 20:11
@ggevay ggevay marked this pull request as ready for review December 2, 2024 20:26
@ggevay ggevay requested review from a team as code owners December 2, 2024 20:26
Copy link
Contributor

@jkosh44 jkosh44 left a comment

Choose a reason for hiding this comment

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

Adapter parts LGTM

@@ -3917,6 +3918,7 @@ impl WithOptionName for ExplainPlanOptionName {
| Self::SubtreeSize
| Self::Timing
| Self::Types
| Self::Equivalences
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add/update a parser test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, done.

Copy link
Contributor

@frankmcsherry frankmcsherry left a comment

Choose a reason for hiding this comment

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

Looks good to me, though without a deep understanding of the explain infra changes.

@ggevay ggevay force-pushed the explain-equivalences branch from 7ee105b to 6dc335e Compare December 3, 2024 09:41
@ggevay ggevay enabled auto-merge December 3, 2024 09:42
@ggevay ggevay merged commit 8e8cccb into MaterializeInc:main Dec 3, 2024
79 checks passed
@def- def- mentioned this pull request Dec 4, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLUSTER Topics related to the CLUSTER layer A-optimization Area: query optimization and transformation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants