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.

Sign up for GitHub

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

Jump to bottom

fix: Xplan validation erroneously throws warning on missing objects #1218

Merged
merged 1 commit into from
Jul 11, 2024
Merged

fix: Xplan validation erroneously throws warning on missing objects #1218

merged 1 commit into from
Jul 11, 2024

Conversation

emanuelaepure10
Copy link
Contributor

Closes #4318

@emanuelaepure10 emanuelaepure10 marked this pull request as ready for review July 10, 2024 08:30
@stempler
Copy link
Member

Related ticket is #890

@stempler
Copy link
Member

@emanuelaepure10 I proposed a change for you to review in a fixup commit.

We had done a change together to try to have the first atomic part in the partitioning for each plan to include the plan. Only the change to first check the plan vertex in the reference graph was not enough, because that first atomic part was then skipped for later processing, since its size exceeded the maximum size desired for a part.
I modified the behavior to not skip the atomic part if the part that is currently prepared is empty. Like that we make sure that the first part retrieved actually contains the plan, and no other parts are being looked at (and thus we don't get any warnings).

emanuelaepure10
emanuelaepure10 commented Jul 11, 2024
View reviewed changes
Copy link
Contributor Author

@emanuelaepure10 emanuelaepure10 left a comment

Choose a reason for hiding this comment

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

LGTM

@stempler stempler added the challenged For PRs to indicate that the implementation has been challenged label Jul 11, 2024
@stempler stempler merged commit 23bcc04 into halestudio:master Jul 11, 2024
8 of 9 checks passed
@stempler stempler deleted the fix/ING-4318 branch July 11, 2024 09:49
@we-release we-release
Copy link

we-release bot commented Aug 7, 2024

🎉 This PR is included in version 5.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@we-release we-release bot added the released label Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stempler stempler stempler approved these changes

Assignees
No one assigned
Labels
challenged For PRs to indicate that the implementation has been challenged released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@emanuelaepure10 @stempler