opgen: added a bool field in struct opgen.transition #90719
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
postamar
approved these changes
Oct 27, 2022
| return tstn.from | ||
| } | ||
| // All transitions results from `equiv(xxx)` specs. Return `to` of the last transition. | ||
| return t.transitions[len(t.transitions)-1].to |
There was a problem hiding this comment.
Wouldn't return target be the same thing here, but simpler?
This PR adds a bool field in struct opgen.transition that indicates
whether it results from a `equiv(xx)` transition spec in the opgen file.
It will be useful for a test where we need to find the inital
status on a adding/dropping path. Without such a change, it can be
problematic if we have a `equiv(xx)` spec as the first transition.
E.g.
ToAbsent(
PUBLIC,
equiv(VALIDATED),
to(WRITE_ONLY),
to(ABSENT),
)
Without this change, the inital status will confusingly be `VALIDATED`,
and the next status will be `PUBLIC`.
With this change, the initial status will be `PUBLIC`, and the next
status will be `WRITE_ONLY`.
We also added some comments when we make transitions from the specs.
Release note: None
Xiang-Gu
force-pushed
the
add-isEquiv-to-transition
branch
from
db2d219
to
bd0bfda
Compare
Xiang-Gu
commented
Oct 31, 2022
There was a problem hiding this comment.
I think it's generally good so I'm going to merge it.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @postamar)
pkg/sql/schemachanger/scplan/internal/opgen/op_gen.go line 154 at r1 (raw file):
Previously, postamar (Marius Posta) wrote…
Wouldn't
return targetbe the same thing here, but simpler?
That's correct! Will change it.
|
TFTR! bors r+ |
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a bool field in struct opgen.transition that indicates whether it results from a
equiv(xx)transition spec in the opgen file. It will be useful for a test where we need to find the inital status on a adding/dropping path. Without such a change, it can be problematic if we have aequiv(xx)spec as the first transition. E.g.Without this change, the inital status will confusingly be
VALIDATED, and the next status will bePUBLIC.With this change, the initial status will be
PUBLIC, and the next status will beWRITE_ONLY.We also added some comments when we make transitions from the specs.
Epic: None
Release note: None