Skip to content

Conversation

@cloud-fan
Copy link
Contributor

What changes were proposed in this pull request?

In PlanStabilitySuite, we normalize expression IDs by matching #\d+ in the explain string. However, this regex can match plan id in Exchange node as well, which will mess up the normalization if expression IDs and plan IDs overlap.

This PR normalizes plan id separately in PlanStabilitySuite.

Why are the changes needed?

Make the plan golden file more stable.

Does this PR introduce any user-facing change?

no

How was this patch tested?

N/A

backport #36827

…ySuite

In `PlanStabilitySuite`, we normalize expression IDs by matching `#\d+` in the explain string. However, this regex can match plan id in `Exchange` node as well, which will mess up the normalization if expression IDs and plan IDs overlap.

This PR normalizes plan id separately in `PlanStabilitySuite`.

Make the plan golden file more stable.

no

N/A

Closes apache#36827 from cloud-fan/test.

Authored-by: Wenchen Fan <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
@github-actions github-actions bot added the SQL label Jun 13, 2022
@cloud-fan
Copy link
Contributor Author

cc @dongjoon-hyun , I've regen-ed plan golden files under ANSI mode as well.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you so much, @cloud-fan .

dongjoon-hyun pushed a commit that referenced this pull request Jun 13, 2022
…bilitySuite

### What changes were proposed in this pull request?

In `PlanStabilitySuite`, we normalize expression IDs by matching `#\d+` in the explain string. However, this regex can match plan id in `Exchange` node as well, which will mess up the normalization if expression IDs and plan IDs overlap.

This PR normalizes plan id separately in `PlanStabilitySuite`.

### Why are the changes needed?

Make the plan golden file more stable.

### Does this PR introduce _any_ user-facing change?

no

### How was this patch tested?

N/A

backport #36827

Closes #36854 from cloud-fan/test2.

Authored-by: Wenchen Fan <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
@dongjoon-hyun
Copy link
Member

Merged to branch-3.3.

@cloud-fan cloud-fan closed this Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants