From bd0bfda74bd41aff5aedfc67cc6768c4cce879c6 Mon Sep 17 00:00:00 2001 From: Xiang Gu Date: Wed, 26 Oct 2022 14:13:06 -0400 Subject: [PATCH] opgen: added a bool field in struct opgen.transition 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 --- .../scplan/internal/opgen/op_gen.go | 9 ++++++++- .../scplan/internal/opgen/target.go | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/op_gen.go b/pkg/sql/schemachanger/scplan/internal/opgen/op_gen.go index 18f253ecdde9..a8f2fa806cc8 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/op_gen.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/op_gen.go @@ -147,7 +147,14 @@ func (r *registry) buildGraph( // InitialStatus returns the status at the source of an op-edge path. func InitialStatus(e scpb.Element, target scpb.Status) scpb.Status { if t, found := findTarget(e, target); found { - return t.transitions[0].from + for _, tstn := range t.transitions { + if tstn.isEquiv { + continue + } + return tstn.from + } + // All transitions results from `equiv(xxx)` specs. Return `to` of the last transition. + return target } return scpb.Status_UNKNOWN } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/target.go b/pkg/sql/schemachanger/scplan/internal/opgen/target.go index ff5600c900e1..6cb1d58282f0 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/target.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/target.go @@ -34,6 +34,7 @@ type transition struct { canFail bool ops opsFunc opType scop.Type + isEquiv bool // True if this transition comes from a `equiv` spec. } func (t transition) OpType() scop.Type { @@ -81,6 +82,20 @@ func makeTarget(e scpb.Element, spec targetSpec) (t target, err error) { return t, nil } +// makeTransitions constructs a slice of transitions from the transition spec, as specified in `opgen_xx.go` file. +// An example can nicely explain how it works. +// Example 1: +// +// Input: toAbsent(PUBLIC, equiv(VALIDATED), to(WRITE_ONLY), to(ABSENT)) +// Output: [VALIDATED, PUBLIC], [PUBLIC, WRITE_ONLY], [WRITE_ONLY, ABSENT] +// +// Example 2: +// +// Input: toPublic(ABSENT, to(DELETE_ONLY), to(WRITE_ONLY), to(PUBLIC)) +// Output: [ABSENT, DELETE_ONLY], [DELETE_ONLY, WRITE_ONLY], [WRITE_ONLY, PUBLIC]. +// +// Right, nothing too surprising except that the trick we use for `equiv(s)` is to encode it +// as a transition from `s` to whatever the current status is. func makeTransitions(e scpb.Element, spec targetSpec) (ret []transition, err error) { tbs := makeTransitionBuildState(spec.from) @@ -92,6 +107,7 @@ func makeTransitions(e scpb.Element, spec targetSpec) (ret []transition, err err for i, s := range spec.transitionSpecs { var t transition if s.from == scpb.Status_UNKNOWN { + // Construct a transition `tbs.from --> s.to`, which comes from a `to(...)` spec. t.from = tbs.from t.to = s.to if err := tbs.withTransition(s, i == 0 /* isFirst */); err != nil { @@ -108,6 +124,7 @@ func makeTransitions(e scpb.Element, spec targetSpec) (ret []transition, err err } } } else { + // Construct a transition `s.from --> tbs.from`, which comes from a `equiv(...)` spec. t.from = s.from t.to = tbs.from if err := tbs.withEquivTransition(s); err != nil { @@ -115,6 +132,7 @@ func makeTransitions(e scpb.Element, spec targetSpec) (ret []transition, err err err, "invalid no-op transition %s -> %s", t.from, t.to, ) } + t.isEquiv = true } t.revertible = tbs.isRevertible if t.opType != scop.MutationType && t.opType != 0 {