diff --git a/pkg/sql/schemachanger/scbuild/builder_state.go b/pkg/sql/schemachanger/scbuild/builder_state.go index bb5707fb0ff8..7a9ff884838e 100644 --- a/pkg/sql/schemachanger/scbuild/builder_state.go +++ b/pkg/sql/schemachanger/scbuild/builder_state.go @@ -68,12 +68,25 @@ func (b *builderState) Ensure(e scpb.Element, target scpb.TargetStatus, meta scp default: panic(errors.AssertionFailedf("unsupported target %s", target.Status())) } + checkForConcurrentSchemaChanges := func() { + // Check that the descriptors relevant to this element are not undergoing + // any concurrent schema changes. + screl.AllTargetDescIDs(e).ForEach(func(id descpb.ID) { + b.ensureDescriptor(id) + if c := b.descCache[id]; c != nil && c.desc != nil && c.desc.HasConcurrentSchemaChanges() { + panic(scerrors.ConcurrentSchemaChangeError(c.desc)) + } + }) + } + if dst == nil { // We're adding both a new element and a target for it. if target == scpb.ToAbsent { // Ignore targets to remove something that doesn't exist yet. return } + // Set a target for the element but check for concurrent schema changes. + checkForConcurrentSchemaChanges() b.addNewElementState(elementState{ element: e, initial: scpb.Status_ABSENT, @@ -89,14 +102,9 @@ func (b *builderState) Ensure(e scpb.Element, target scpb.TargetStatus, meta scp return } - // Check that the descriptors relevant to this element are not undergoing any - // concurrent schema changes. - screl.AllTargetDescIDs(e).ForEach(func(id descpb.ID) { - b.ensureDescriptor(id) - if c := b.descCache[id]; c != nil && c.desc != nil && c.desc.HasConcurrentSchemaChanges() { - panic(scerrors.ConcurrentSchemaChangeError(c.desc)) - } - }) + // Check that there are no concurrent schema changes on the descriptors + // referenced by this element. + checkForConcurrentSchemaChanges() // Re-assign dst because the above function may have mutated the builder // state. Specifically, the output slice, to which dst points to, might // have grown and might have been reallocated. diff --git a/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_udf/alter_table_add_check_udf.side_effects b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_udf/alter_table_add_check_udf.side_effects index 33e4976a4070..76c944dff14a 100644 --- a/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_udf/alter_table_add_check_udf.side_effects +++ b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_udf/alter_table_add_check_udf.side_effects @@ -162,7 +162,9 @@ upsert descriptor #105 + authorization: + userName: root + jobId: "1" - + nameMapping: {} + + nameMapping: + + id: 105 + + name: f + revertible: true + dependedOnBy: + - constraintIds: @@ -262,7 +264,9 @@ upsert descriptor #105 - authorization: - userName: root - jobId: "1" - - nameMapping: {} + - nameMapping: + - id: 105 + - name: f - revertible: true dependedOnBy: - constraintIds: diff --git a/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt.explain b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt.explain index 542e57b37abc..cab790d5109d 100644 --- a/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt.explain +++ b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt.explain @@ -10,7 +10,7 @@ Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CHEC ├── StatementPhase │ └── Stage 1 of 1 in StatementPhase │ ├── 2 elements transitioning toward PUBLIC - │ │ ├── ABSENT → WRITE_ONLY CheckConstraint:{DescID: 107 (t), ReferencedTypeIDs: [105 (typ), 106 (#106)], IndexID: 0, ConstraintID: 2 (check_i_j+), ReferencedSequenceIDs: [104 (s)]} + │ │ ├── ABSENT → WRITE_ONLY CheckConstraint:{DescID: 107 (t), ReferencedTypeIDs: [105 (typ), 106 (_typ)], IndexID: 0, ConstraintID: 2 (check_i_j+), ReferencedSequenceIDs: [104 (s)]} │ │ └── ABSENT → PUBLIC ConstraintWithoutIndexName:{DescID: 107 (t), Name: "check_i_j", ConstraintID: 2 (check_i_j+)} │ └── 4 Mutation operations │ ├── AddCheckConstraint {"CheckExpr":"(i \u003e nextval(104...","ConstraintID":2,"TableID":107,"Validity":2} @@ -20,13 +20,13 @@ Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CHEC ├── PreCommitPhase │ ├── Stage 1 of 2 in PreCommitPhase │ │ ├── 2 elements transitioning toward PUBLIC - │ │ │ ├── WRITE_ONLY → ABSENT CheckConstraint:{DescID: 107 (t), ReferencedTypeIDs: [105 (typ), 106 (#106)], IndexID: 0, ConstraintID: 2 (check_i_j+), ReferencedSequenceIDs: [104 (s)]} + │ │ │ ├── WRITE_ONLY → ABSENT CheckConstraint:{DescID: 107 (t), ReferencedTypeIDs: [105 (typ), 106 (_typ)], IndexID: 0, ConstraintID: 2 (check_i_j+), ReferencedSequenceIDs: [104 (s)]} │ │ │ └── PUBLIC → ABSENT ConstraintWithoutIndexName:{DescID: 107 (t), Name: "check_i_j", ConstraintID: 2 (check_i_j+)} │ │ └── 1 Mutation operation │ │ └── UndoAllInTxnImmediateMutationOpSideEffects │ └── Stage 2 of 2 in PreCommitPhase │ ├── 2 elements transitioning toward PUBLIC - │ │ ├── ABSENT → WRITE_ONLY CheckConstraint:{DescID: 107 (t), ReferencedTypeIDs: [105 (typ), 106 (#106)], IndexID: 0, ConstraintID: 2 (check_i_j+), ReferencedSequenceIDs: [104 (s)]} + │ │ ├── ABSENT → WRITE_ONLY CheckConstraint:{DescID: 107 (t), ReferencedTypeIDs: [105 (typ), 106 (_typ)], IndexID: 0, ConstraintID: 2 (check_i_j+), ReferencedSequenceIDs: [104 (s)]} │ │ └── ABSENT → PUBLIC ConstraintWithoutIndexName:{DescID: 107 (t), Name: "check_i_j", ConstraintID: 2 (check_i_j+)} │ └── 9 Mutation operations │ ├── AddCheckConstraint {"CheckExpr":"(i \u003e nextval(104...","ConstraintID":2,"TableID":107,"Validity":2} @@ -41,12 +41,12 @@ Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CHEC └── PostCommitPhase ├── Stage 1 of 2 in PostCommitPhase │ ├── 1 element transitioning toward PUBLIC - │ │ └── WRITE_ONLY → VALIDATED CheckConstraint:{DescID: 107 (t), ReferencedTypeIDs: [105 (typ), 106 (#106)], IndexID: 0, ConstraintID: 2 (check_i_j+), ReferencedSequenceIDs: [104 (s)]} + │ │ └── WRITE_ONLY → VALIDATED CheckConstraint:{DescID: 107 (t), ReferencedTypeIDs: [105 (typ), 106 (_typ)], IndexID: 0, ConstraintID: 2 (check_i_j+), ReferencedSequenceIDs: [104 (s)]} │ └── 1 Validation operation │ └── ValidateConstraint {"ConstraintID":2,"TableID":107} └── Stage 2 of 2 in PostCommitPhase ├── 1 element transitioning toward PUBLIC - │ └── VALIDATED → PUBLIC CheckConstraint:{DescID: 107 (t), ReferencedTypeIDs: [105 (typ), 106 (#106)], IndexID: 0, ConstraintID: 2 (check_i_j+), ReferencedSequenceIDs: [104 (s)]} + │ └── VALIDATED → PUBLIC CheckConstraint:{DescID: 107 (t), ReferencedTypeIDs: [105 (typ), 106 (_typ)], IndexID: 0, ConstraintID: 2 (check_i_j+), ReferencedSequenceIDs: [104 (s)]} └── 6 Mutation operations ├── MakeValidatedCheckConstraintPublic {"ConstraintID":2,"TableID":107} ├── RemoveJobStateFromDescriptor {"DescriptorID":104} diff --git a/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt.side_effects b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt.side_effects index c79576466884..237be13e28da 100644 --- a/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt.side_effects +++ b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt.side_effects @@ -155,7 +155,9 @@ upsert descriptor #106 + authorization: + userName: root + jobId: "1" - + nameMapping: {} + + nameMapping: + + id: 106 + + name: _typ + revertible: true id: 106 kind: ALIAS @@ -301,7 +303,9 @@ upsert descriptor #106 - authorization: - userName: root - jobId: "1" - - nameMapping: {} + - nameMapping: + - id: 106 + - name: _typ - revertible: true id: 106 kind: ALIAS diff --git a/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt__rollback_1_of_2.explain b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt__rollback_1_of_2.explain index 484f28e95fb6..738ce111b0b8 100644 --- a/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt__rollback_1_of_2.explain +++ b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt__rollback_1_of_2.explain @@ -11,7 +11,7 @@ Schema change plan for rolling back ALTER TABLE ‹defaultdb›.public.‹t› A └── PostCommitNonRevertiblePhase └── Stage 1 of 1 in PostCommitNonRevertiblePhase ├── 2 elements transitioning toward ABSENT - │ ├── WRITE_ONLY → ABSENT CheckConstraint:{DescID: 107 (t), ReferencedTypeIDs: [105 (typ), 106 (#106)], IndexID: 0, ConstraintID: 2 (check_i_j-), ReferencedSequenceIDs: [104 (s)]} + │ ├── WRITE_ONLY → ABSENT CheckConstraint:{DescID: 107 (t), ReferencedTypeIDs: [105 (typ), 106 (_typ)], IndexID: 0, ConstraintID: 2 (check_i_j-), ReferencedSequenceIDs: [104 (s)]} │ └── PUBLIC → ABSENT ConstraintWithoutIndexName:{DescID: 107 (t), Name: "check_i_j", ConstraintID: 2 (check_i_j-)} └── 9 Mutation operations ├── SetConstraintName {"ConstraintID":2,"Name":"crdb_internal_co...","TableID":107} diff --git a/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt__rollback_2_of_2.explain b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt__rollback_2_of_2.explain index 77f62eb66577..c6c023e1d6ef 100644 --- a/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt__rollback_2_of_2.explain +++ b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_with_seq_and_udt/alter_table_add_check_with_seq_and_udt__rollback_2_of_2.explain @@ -11,7 +11,7 @@ Schema change plan for rolling back ALTER TABLE ‹defaultdb›.public.‹t› A └── PostCommitNonRevertiblePhase └── Stage 1 of 1 in PostCommitNonRevertiblePhase ├── 2 elements transitioning toward ABSENT - │ ├── WRITE_ONLY → ABSENT CheckConstraint:{DescID: 107 (t), ReferencedTypeIDs: [105 (typ), 106 (#106)], IndexID: 0, ConstraintID: 2 (check_i_j-), ReferencedSequenceIDs: [104 (s)]} + │ ├── WRITE_ONLY → ABSENT CheckConstraint:{DescID: 107 (t), ReferencedTypeIDs: [105 (typ), 106 (_typ)], IndexID: 0, ConstraintID: 2 (check_i_j-), ReferencedSequenceIDs: [104 (s)]} │ └── PUBLIC → ABSENT ConstraintWithoutIndexName:{DescID: 107 (t), Name: "check_i_j", ConstraintID: 2 (check_i_j-)} └── 9 Mutation operations ├── SetConstraintName {"ConstraintID":2,"Name":"crdb_internal_co...","TableID":107} diff --git a/pkg/sql/schemachanger/testdata/end_to_end/create_index/create_index.side_effects b/pkg/sql/schemachanger/testdata/end_to_end/create_index/create_index.side_effects index 5d486d1d5823..f10d7f1134ac 100644 --- a/pkg/sql/schemachanger/testdata/end_to_end/create_index/create_index.side_effects +++ b/pkg/sql/schemachanger/testdata/end_to_end/create_index/create_index.side_effects @@ -135,7 +135,9 @@ upsert descriptor #105 + authorization: + userName: root + jobId: "1" - + nameMapping: {} + + nameMapping: + + id: 105 + + name: _e + revertible: true id: 105 kind: ALIAS @@ -420,7 +422,9 @@ upsert descriptor #105 - authorization: - userName: root - jobId: "1" - - nameMapping: {} + - nameMapping: + - id: 105 + - name: _e - revertible: true id: 105 kind: ALIAS