Skip to content

Commit dabb49e

Browse files
committed
sql/schemachanger: adopt schema_locked by default in declarative tests
Previously, there were individual tests for confirming if schema_locked worked correctly in the declarative schema changer. This patch flips create_table_with_schema_locked and update tests to handle this. Specifically it does the following: 1) All tables are created as schema_locked in the declarative schema changer tests 2) All setup statements are now run via the declarative schema changer so that schema_locked is almost never manually toggled 3) Updates all test outputs to expected schema_locked Informs: #129694 Release note: None
1 parent c1cf511 commit dabb49e

File tree

843 files changed

+22554
-19063
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

843 files changed

+22554
-19063
lines changed

Diff for: pkg/ccl/schemachangerccl/BUILD.bazel

+2
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,12 @@ go_test(
3434
"//pkg/base",
3535
"//pkg/ccl",
3636
"//pkg/ccl/multiregionccl/multiregionccltestutils",
37+
"//pkg/clusterversion",
3738
"//pkg/jobs",
3839
"//pkg/security/securityassets",
3940
"//pkg/security/securitytest",
4041
"//pkg/server",
42+
"//pkg/settings/cluster",
4143
"//pkg/sql",
4244
"//pkg/sql/schemachanger/scexec",
4345
"//pkg/sql/schemachanger/sctest",

Diff for: pkg/ccl/schemachangerccl/backup_base_generated_test.go

-56
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: pkg/ccl/schemachangerccl/schemachanger_ccl_test.go

+18-3
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@ import (
1313

1414
"github.com/cockroachdb/cockroach/pkg/base"
1515
"github.com/cockroachdb/cockroach/pkg/ccl/multiregionccl/multiregionccltestutils"
16+
"github.com/cockroachdb/cockroach/pkg/clusterversion"
1617
"github.com/cockroachdb/cockroach/pkg/jobs"
1718
"github.com/cockroachdb/cockroach/pkg/server"
19+
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
1820
"github.com/cockroachdb/cockroach/pkg/sql"
1921
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scexec"
2022
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/sctest"
@@ -29,8 +31,9 @@ import (
2931
// MultiRegionTestClusterFactory is a multi-region implementation of the
3032
// sctest.TestServerFactory interface.
3133
type MultiRegionTestClusterFactory struct {
32-
scexec *scexec.TestingKnobs
33-
server *server.TestingKnobs
34+
scexec *scexec.TestingKnobs
35+
server *server.TestingKnobs
36+
schemaLockedDisabled bool
3437
}
3538

3639
var _ sctest.TestServerFactory = MultiRegionTestClusterFactory{}
@@ -52,6 +55,12 @@ func (f MultiRegionTestClusterFactory) WithMixedVersion() sctest.TestServerFacto
5255
return f
5356
}
5457

58+
// WithSchemaLockDisabled implements the sctest.TestServerFactory interface.
59+
func (f MultiRegionTestClusterFactory) WithSchemaLockDisabled() sctest.TestServerFactory {
60+
f.schemaLockedDisabled = true
61+
return f
62+
}
63+
5564
// Run implements the sctest.TestServerFactory interface.
5665
func (f MultiRegionTestClusterFactory) Run(
5766
ctx context.Context, t *testing.T, fn func(_ serverutils.TestServerInterface, _ *gosql.DB),
@@ -69,7 +78,13 @@ func (f MultiRegionTestClusterFactory) Run(
6978
if f.scexec != nil {
7079
knobs.SQLDeclarativeSchemaChanger = f.scexec
7180
}
72-
c, db, _ := multiregionccltestutils.TestingCreateMultiRegionCluster(t, numServers, knobs)
81+
// Always run this test with schema_locked by default.
82+
st := cluster.MakeTestingClusterSettings()
83+
if f.server != nil && f.server.ClusterVersionOverride.Major != 0 {
84+
st = cluster.MakeClusterSettingsWithVersions(clusterversion.Latest.Version(), f.server.ClusterVersionOverride)
85+
}
86+
sql.CreateTableWithSchemaLocked.Override(ctx, &st.SV, !f.schemaLockedDisabled)
87+
c, db, _ := multiregionccltestutils.TestingCreateMultiRegionCluster(t, numServers, knobs, multiregionccltestutils.WithSettings(st))
7388
defer c.Stopper().Stop(ctx)
7489
fn(c.Server(0), db)
7590
}

Diff for: pkg/ccl/schemachangerccl/testdata/decomp/multiregion

+9
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,9 @@ ElementState:
475475
- TableLocalityGlobal:
476476
tableId: 110
477477
Status: PUBLIC
478+
- TableSchemaLocked:
479+
tableId: 110
480+
Status: PUBLIC
478481
- TableZoneConfig:
479482
seqNum: 0
480483
tableId: 110
@@ -874,6 +877,9 @@ ElementState:
874877
regionName: us-east2
875878
tableId: 109
876879
Status: PUBLIC
880+
- TableSchemaLocked:
881+
tableId: 109
882+
Status: PUBLIC
877883
- TableZoneConfig:
878884
seqNum: 0
879885
tableId: 109
@@ -1571,6 +1577,9 @@ ElementState:
15711577
- TablePartitioning:
15721578
tableId: 108
15731579
Status: PUBLIC
1580+
- TableSchemaLocked:
1581+
tableId: 108
1582+
Status: PUBLIC
15741583
- TableZoneConfig:
15751584
seqNum: 0
15761585
tableId: 108

Diff for: pkg/ccl/schemachangerccl/testdata/decomp/partitioning

+6
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,9 @@ ElementState:
545545
databaseId: 100
546546
tableId: 104
547547
Status: PUBLIC
548+
- TableSchemaLocked:
549+
tableId: 104
550+
Status: PUBLIC
548551
- UserPrivileges:
549552
descriptorId: 104
550553
privileges: "2"
@@ -977,6 +980,9 @@ ElementState:
977980
databaseId: 100
978981
tableId: 105
979982
Status: PUBLIC
983+
- TableSchemaLocked:
984+
tableId: 105
985+
Status: PUBLIC
980986
- UserPrivileges:
981987
descriptorId: 105
982988
privileges: "2"

Diff for: pkg/ccl/schemachangerccl/testdata/end_to_end/add_column_regional_by_row/add_column_regional_by_row.explain

+29-14
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@ Schema change plan for ALTER TABLE ‹multiregion_db›.‹public›.‹table_re
3030
│ │ ├── ABSENT → PUBLIC IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 1 (k), IndexID: 3}
3131
│ │ ├── ABSENT → PUBLIC IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 2 (v), IndexID: 3}
3232
│ │ └── ABSENT → PUBLIC IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 4 (w+), IndexID: 3}
33-
│ └── 16 Mutation operations
33+
│ ├── 1 element transitioning toward TRANSIENT_PUBLIC
34+
│ │ └── PUBLIC → ABSENT TableSchemaLocked:{DescID: 108 (table_regional_by_row)}
35+
│ └── 17 Mutation operations
36+
│ ├── SetTableSchemaLocked {"TableID":108}
3437
│ ├── MakeAbsentColumnDeleteOnly {"Column":{"ColumnID":4,"TableID":108}}
3538
│ ├── SetColumnName {"ColumnID":4,"Name":"w","TableID":108}
3639
│ ├── UpsertColumnType {"ColumnType":{"ColumnID":4,"TableID":108}}
@@ -68,6 +71,8 @@ Schema change plan for ALTER TABLE ‹multiregion_db›.‹public›.‹table_re
6871
│ │ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 1 (k), IndexID: 3}
6972
│ │ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 2 (v), IndexID: 3}
7073
│ │ │ └── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 4 (w+), IndexID: 3}
74+
│ │ ├── 1 element transitioning toward TRANSIENT_PUBLIC
75+
│ │ │ └── ABSENT → PUBLIC TableSchemaLocked:{DescID: 108 (table_regional_by_row)}
7176
│ │ └── 1 Mutation operation
7277
│ │ └── UndoAllInTxnImmediateMutationOpSideEffects
7378
│ └── Stage 2 of 2 in PreCommitPhase
@@ -90,7 +95,10 @@ Schema change plan for ALTER TABLE ‹multiregion_db›.‹public›.‹table_re
9095
│ │ ├── ABSENT → PUBLIC IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 1 (k), IndexID: 3}
9196
│ │ ├── ABSENT → PUBLIC IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 2 (v), IndexID: 3}
9297
│ │ └── ABSENT → PUBLIC IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 4 (w+), IndexID: 3}
93-
│ └── 20 Mutation operations
98+
│ ├── 1 element transitioning toward TRANSIENT_PUBLIC
99+
│ │ └── PUBLIC → ABSENT TableSchemaLocked:{DescID: 108 (table_regional_by_row)}
100+
│ └── 21 Mutation operations
101+
│ ├── SetTableSchemaLocked {"TableID":108}
94102
│ ├── MakeAbsentColumnDeleteOnly {"Column":{"ColumnID":4,"TableID":108}}
95103
│ ├── SetColumnName {"ColumnID":4,"Name":"w","TableID":108}
96104
│ ├── UpsertColumnType {"ColumnType":{"ColumnID":4,"TableID":108}}
@@ -167,7 +175,7 @@ Schema change plan for ALTER TABLE ‹multiregion_db›.‹public›.‹table_re
167175
│ ├── ValidateIndex {"IndexID":2,"TableID":108}
168176
│ └── ValidateColumnNotNull {"ColumnID":4,"IndexIDForValidation":2,"TableID":108}
169177
└── PostCommitNonRevertiblePhase
170-
├── Stage 1 of 3 in PostCommitNonRevertiblePhase
178+
├── Stage 1 of 4 in PostCommitNonRevertiblePhase
171179
│ ├── 4 elements transitioning toward PUBLIC
172180
│ │ ├── WRITE_ONLY → PUBLIC Column:{DescID: 108 (table_regional_by_row), ColumnID: 4 (w+)}
173181
│ │ ├── VALIDATED → PUBLIC PrimaryIndex:{DescID: 108 (table_regional_by_row), IndexID: 2 (table_regional_by_row_pkey+), ConstraintID: 2, TemporaryIndexID: 3, SourceIndexID: 1 (table_regional_by_row_pkey-)}
@@ -198,7 +206,7 @@ Schema change plan for ALTER TABLE ‹multiregion_db›.‹public›.‹table_re
198206
│ ├── RefreshStats {"TableID":108}
199207
│ ├── SetJobStateOnDescriptor {"DescriptorID":108}
200208
│ └── UpdateSchemaChangerJob {"IsNonCancelable":true,"RunningStatus":"PostCommitNonRev..."}
201-
├── Stage 2 of 3 in PostCommitNonRevertiblePhase
209+
├── Stage 2 of 4 in PostCommitNonRevertiblePhase
202210
│ ├── 4 elements transitioning toward ABSENT
203211
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 3 (crdb_region), IndexID: 1 (table_regional_by_row_pkey-)}
204212
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 1 (k), IndexID: 1 (table_regional_by_row_pkey-)}
@@ -211,15 +219,22 @@ Schema change plan for ALTER TABLE ‹multiregion_db›.‹public›.‹table_re
211219
│ ├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":1,"Kind":2,"TableID":108}
212220
│ ├── SetJobStateOnDescriptor {"DescriptorID":108}
213221
│ └── UpdateSchemaChangerJob {"IsNonCancelable":true,"RunningStatus":"PostCommitNonRev..."}
214-
└── Stage 3 of 3 in PostCommitNonRevertiblePhase
215-
├── 1 element transitioning toward TRANSIENT_ABSENT
216-
│ └── PUBLIC → TRANSIENT_ABSENT IndexData:{DescID: 108 (table_regional_by_row), IndexID: 3}
217-
├── 2 elements transitioning toward ABSENT
218-
│ ├── DELETE_ONLY → ABSENT PrimaryIndex:{DescID: 108 (table_regional_by_row), IndexID: 1 (table_regional_by_row_pkey-), ConstraintID: 1}
219-
│ └── PUBLIC → ABSENT IndexData:{DescID: 108 (table_regional_by_row), IndexID: 1 (table_regional_by_row_pkey-)}
220-
└── 5 Mutation operations
221-
├── MakeIndexAbsent {"IndexID":1,"TableID":108}
222-
├── CreateGCJobForIndex {"IndexID":1,"TableID":108}
223-
├── CreateGCJobForIndex {"IndexID":3,"TableID":108}
222+
├── Stage 3 of 4 in PostCommitNonRevertiblePhase
223+
│ ├── 1 element transitioning toward TRANSIENT_ABSENT
224+
│ │ └── PUBLIC → TRANSIENT_ABSENT IndexData:{DescID: 108 (table_regional_by_row), IndexID: 3}
225+
│ ├── 2 elements transitioning toward ABSENT
226+
│ │ ├── DELETE_ONLY → ABSENT PrimaryIndex:{DescID: 108 (table_regional_by_row), IndexID: 1 (table_regional_by_row_pkey-), ConstraintID: 1}
227+
│ │ └── PUBLIC → ABSENT IndexData:{DescID: 108 (table_regional_by_row), IndexID: 1 (table_regional_by_row_pkey-)}
228+
│ └── 5 Mutation operations
229+
│ ├── MakeIndexAbsent {"IndexID":1,"TableID":108}
230+
│ ├── CreateGCJobForIndex {"IndexID":1,"TableID":108}
231+
│ ├── CreateGCJobForIndex {"IndexID":3,"TableID":108}
232+
│ ├── SetJobStateOnDescriptor {"DescriptorID":108}
233+
│ └── UpdateSchemaChangerJob {"IsNonCancelable":true,"RunningStatus":"PostCommitNonRev..."}
234+
└── Stage 4 of 4 in PostCommitNonRevertiblePhase
235+
├── 1 element transitioning toward TRANSIENT_PUBLIC
236+
│ └── ABSENT → TRANSIENT_PUBLIC TableSchemaLocked:{DescID: 108 (table_regional_by_row)}
237+
└── 3 Mutation operations
238+
├── SetTableSchemaLocked {"Locked":true,"TableID":108}
224239
├── RemoveJobStateFromDescriptor {"DescriptorID":108}
225240
└── UpdateSchemaChangerJob {"IsNonCancelable":true,"RunningStatus":"all stages compl..."}

Diff for: pkg/ccl/schemachangerccl/testdata/end_to_end/add_column_regional_by_row/add_column_regional_by_row.explain_shape

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,4 @@ Schema change plan for ALTER TABLE ‹multiregion_db›.‹public›.‹table_re
1818
├── execute 1 system table mutations transaction
1919
├── validate UNIQUE constraint backed by index table_regional_by_row_pkey+ in relation table_regional_by_row
2020
├── validate NOT NULL constraint on column w+ in index table_regional_by_row_pkey+ in relation table_regional_by_row
21-
└── execute 3 system table mutations transactions
21+
└── execute 4 system table mutations transactions

0 commit comments

Comments
 (0)