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

objchange: set length is unknown with partially known elements #33377

Merged
merged 1 commit into from Jun 15, 2023

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Jun 15, 2023

If a set contains partially known values Length will be unknown which causes assertPlannedObjectValid to fail valid plans.

Revert to the old method if using LengthInt for the set lengths, which returns the maximum number of possible elements, with a guard for entirely unknown set values. Improve the check for incorrectly computed objects to catch existing test cases.

While technically inaccurate, LengthInt values are not going to fail for valid plans and gives us a minimal fix which can be backported to v1.5. A follow up PR for the development branch will contain a more precise fix using value refinements.

fixes #33371

If a set contains partially known values the length is unknown which
causes assertPlannedObjectValid to fail valid plans.

Revert to the old method if using LengthInt for the set lengths, which
returns the maximum number of possible elements, with a guard for
entirely unknown set values.
@jbardin jbardin added the 1.5-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Jun 15, 2023
@jbardin jbardin requested a review from a team June 15, 2023 13:39
Copy link
Member

@apparentlymart apparentlymart left a comment

Choose a reason for hiding this comment

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

Seems plausible as an interim 1.5 fix!

@jbardin jbardin merged commit 0be4a38 into main Jun 15, 2023
6 checks passed
@jbardin jbardin deleted the jbardin/nesting-set-length branch June 15, 2023 14:26
@github-actions
Copy link

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1.5-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SetNested objects with Computed attributes produce plan errors in 1.5.0
2 participants