schemachanger: fix upgrade jobs to work with declarative schema changer

Now that the internal executor is using a non-zero session data, that means it uses the declarative schema changer by default. The upgrade manager had a few assumptions in it that were specific to the old schema changer: - The PGAttributeNum field should only be set if it differs from the column descriptor ID. - Check the DeclarativeSchemaChangerState when waiting for migrations to finish. Release note: None
cockroachdb · Apr 15, 2023 · 236414b · 236414b
1 parent 07cc6d4
commit 236414b
Show file tree

Hide file tree

Showing 52 changed files with 255 additions and 305 deletions.
diff --git a/pkg/ccl/logictestccl/testdata/logic_test/redact_descriptor b/pkg/ccl/logictestccl/testdata/logic_test/redact_descriptor
@@ -75,7 +75,6 @@ SELECT descriptor from redacted_descriptors where id = 'collate_partition'::REGC
                 "id": 2,
                 "name": "b",
                 "nullable": true,
-                "pgAttributeNum": 2,
                 "type": {
                     "family": "StringFamily",
                     "oid": 25

diff --git a/pkg/ccl/schemachangerccl/testdata/decomp/multiregion b/pkg/ccl/schemachangerccl/testdata/decomp/multiregion
@@ -88,7 +88,7 @@ ElementState:
     isHidden: false
     isInaccessible: false
     isSystemColumn: false
-    pgAttributeNum: 1
+    pgAttributeNum: 0
     tableId: 110
   Status: PUBLIC
 - Column:
@@ -98,7 +98,7 @@ ElementState:
     isHidden: true
     isInaccessible: false
     isSystemColumn: false
-    pgAttributeNum: 2
+    pgAttributeNum: 0
     tableId: 110
   Status: PUBLIC
 - Column:
@@ -108,7 +108,7 @@ ElementState:
     isHidden: true
     isInaccessible: false
     isSystemColumn: true
-    pgAttributeNum: 4.294967294e+09
+    pgAttributeNum: 0
     tableId: 110
   Status: PUBLIC
 - Column:
@@ -118,7 +118,7 @@ ElementState:
     isHidden: true
     isInaccessible: false
     isSystemColumn: true
-    pgAttributeNum: 4.294967295e+09
+    pgAttributeNum: 0
     tableId: 110
   Status: PUBLIC
 - PrimaryIndex:
@@ -365,7 +365,7 @@ ElementState:
     isHidden: false
     isInaccessible: false
     isSystemColumn: false
-    pgAttributeNum: 1
+    pgAttributeNum: 0
     tableId: 109
   Status: PUBLIC
 - Column:
@@ -375,7 +375,7 @@ ElementState:
     isHidden: true
     isInaccessible: false
     isSystemColumn: false
-    pgAttributeNum: 2
+    pgAttributeNum: 0
     tableId: 109
   Status: PUBLIC
 - Column:
@@ -385,7 +385,7 @@ ElementState:
     isHidden: true
     isInaccessible: false
     isSystemColumn: true
-    pgAttributeNum: 4.294967294e+09
+    pgAttributeNum: 0
     tableId: 109
   Status: PUBLIC
 - Column:
@@ -395,7 +395,7 @@ ElementState:
     isHidden: true
     isInaccessible: false
     isSystemColumn: true
-    pgAttributeNum: 4.294967295e+09
+    pgAttributeNum: 0
     tableId: 109
   Status: PUBLIC
 - PrimaryIndex:
@@ -644,7 +644,7 @@ ElementState:
     isHidden: false
     isInaccessible: false
     isSystemColumn: false
-    pgAttributeNum: 1
+    pgAttributeNum: 0
     tableId: 108
   Status: PUBLIC
 - Column:
@@ -654,7 +654,7 @@ ElementState:
     isHidden: false
     isInaccessible: false
     isSystemColumn: false
-    pgAttributeNum: 2
+    pgAttributeNum: 0
     tableId: 108
   Status: PUBLIC
 - Column:
@@ -664,7 +664,7 @@ ElementState:
     isHidden: true
     isInaccessible: false
     isSystemColumn: false
-    pgAttributeNum: 3
+    pgAttributeNum: 0
     tableId: 108
   Status: PUBLIC
 - Column:
@@ -674,7 +674,7 @@ ElementState:
     isHidden: true
     isInaccessible: false
     isSystemColumn: true
-    pgAttributeNum: 4.294967294e+09
+    pgAttributeNum: 0
     tableId: 108
   Status: PUBLIC
 - Column:
@@ -684,7 +684,7 @@ ElementState:
     isHidden: true
     isInaccessible: false
     isSystemColumn: true
-    pgAttributeNum: 4.294967295e+09
+    pgAttributeNum: 0
     tableId: 108
   Status: PUBLIC
 - PrimaryIndex:

diff --git a/pkg/ccl/schemachangerccl/testdata/decomp/partitioning b/pkg/ccl/schemachangerccl/testdata/decomp/partitioning
@@ -45,7 +45,7 @@ ElementState:
     isHidden: false
     isInaccessible: false
     isSystemColumn: false
-    pgAttributeNum: 1
+    pgAttributeNum: 0
     tableId: 104
   Status: PUBLIC
 - Column:
@@ -55,7 +55,7 @@ ElementState:
     isHidden: false
     isInaccessible: false
     isSystemColumn: false
-    pgAttributeNum: 2
+    pgAttributeNum: 0
     tableId: 104
   Status: PUBLIC
 - Column:
@@ -65,7 +65,7 @@ ElementState:
     isHidden: false
     isInaccessible: false
     isSystemColumn: false
-    pgAttributeNum: 3
+    pgAttributeNum: 0
     tableId: 104
   Status: PUBLIC
 - Column:
@@ -75,7 +75,7 @@ ElementState:
     isHidden: true
     isInaccessible: false
     isSystemColumn: true
-    pgAttributeNum: 4.294967294e+09
+    pgAttributeNum: 0
     tableId: 104
   Status: PUBLIC
 - Column:
@@ -85,7 +85,7 @@ ElementState:
     isHidden: true
     isInaccessible: false
     isSystemColumn: true
-    pgAttributeNum: 4.294967295e+09
+    pgAttributeNum: 0
     tableId: 104
   Status: PUBLIC
 - PrimaryIndex:
@@ -468,7 +468,7 @@ ElementState:
     isHidden: false
     isInaccessible: false
     isSystemColumn: false
-    pgAttributeNum: 1
+    pgAttributeNum: 0
     tableId: 105
   Status: PUBLIC
 - Column:
@@ -478,7 +478,7 @@ ElementState:
     isHidden: false
     isInaccessible: false
     isSystemColumn: false
-    pgAttributeNum: 2
+    pgAttributeNum: 0
     tableId: 105
   Status: PUBLIC
 - Column:
@@ -488,7 +488,7 @@ ElementState:
     isHidden: true
     isInaccessible: false
     isSystemColumn: true
-    pgAttributeNum: 4.294967294e+09
+    pgAttributeNum: 0
     tableId: 105
   Status: PUBLIC
 - Column:
@@ -498,7 +498,7 @@ ElementState:
     isHidden: true
     isInaccessible: false
     isSystemColumn: true
-    pgAttributeNum: 4.294967295e+09
+    pgAttributeNum: 0
     tableId: 105
   Status: PUBLIC
 - PrimaryIndex:

diff --git a/pkg/sql/logictest/testdata/logic_test/expression_index b/pkg/sql/logictest/testdata/logic_test/expression_index
@@ -267,7 +267,7 @@ SELECT * FROM (
   ) AS desc FROM system.descriptor WHERE id = 't'::REGCLASS
 ) AS cols WHERE cols.desc->'name' = '"crdb_internal_idx_expr_2"'
 ----
-{"computeExpr": "a + 10:::INT8", "id": 9, "inaccessible": true, "name": "crdb_internal_idx_expr_2", "nullable": true, "pgAttributeNum": 9, "type": {"family": "IntFamily", "oid": 20, "width": 64}, "virtual": true}
+{"computeExpr": "a + 10:::INT8", "id": 9, "inaccessible": true, "name": "crdb_internal_idx_expr_2", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}, "virtual": true}
 
 statement ok
 DROP INDEX t_lower_c_a_plus_b_idx
@@ -284,8 +284,8 @@ SELECT * FROM (
   ) AS desc FROM system.descriptor WHERE id = 't'::REGCLASS
 ) AS cols WHERE cols.desc->>'name' IN ('crdb_internal_idx_expr', 'crdb_internal_idx_expr_1')
 ----
-{"computeExpr": "a + b", "id": 7, "inaccessible": true, "name": "crdb_internal_idx_expr", "nullable": true, "pgAttributeNum": 7, "type": {"family": "IntFamily", "oid": 20, "width": 64}, "virtual": true}
-{"computeExpr": "lower(c)", "id": 8, "inaccessible": true, "name": "crdb_internal_idx_expr_1", "nullable": true, "pgAttributeNum": 8, "type": {"family": "StringFamily", "oid": 25}, "virtual": true}
+{"computeExpr": "a + b", "id": 7, "inaccessible": true, "name": "crdb_internal_idx_expr", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}, "virtual": true}
+{"computeExpr": "lower(c)", "id": 8, "inaccessible": true, "name": "crdb_internal_idx_expr_1", "nullable": true, "type": {"family": "StringFamily", "oid": 25}, "virtual": true}
 
 onlyif config local-legacy-schema-changer
 query T

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/show_trace b/pkg/sql/opt/exec/execbuilder/testdata/show_trace
diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_column.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_column.go
@@ -29,6 +29,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scdecomp"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scerrors"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
@@ -123,11 +124,14 @@ func alterTableAddColumn(
 			IsHidden:                desc.Hidden,
 			IsInaccessible:          desc.Inaccessible,
 			GeneratedAsIdentityType: desc.GeneratedAsIdentityType,
-			PgAttributeNum:          desc.GetPGAttributeNum(),
 		},
 		unique:  d.Unique.IsUnique,
 		notNull: !desc.Nullable,
 	}
+	// Only set PgAttributeNum if it differs from ColumnID.
+	if pgAttNum := desc.GetPGAttributeNum(); pgAttNum != catid.PGAttributeNum(desc.ID) {
+		spec.col.PgAttributeNum = pgAttNum
+	}
 	if ptr := desc.GeneratedAsIdentitySequenceOption; ptr != nil {
 		spec.col.GeneratedAsIdentitySequenceOption = *ptr
 	}

diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_index.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_index.go
@@ -707,10 +707,9 @@ func maybeCreateAndAddShardCol(
 	spec := addColumnSpec{
 		tbl: tbl,
 		col: &scpb.Column{
-			TableID:        tbl.TableID,
-			ColumnID:       shardColID,
-			IsHidden:       true,
-			PgAttributeNum: catid.PGAttributeNum(shardColID),
+			TableID:  tbl.TableID,
+			ColumnID: shardColID,
+			IsHidden: true,
 		},
 		name: &scpb.ColumnName{
 			TableID:  tbl.TableID,

diff --git a/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_column b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_column
@@ -6,7 +6,7 @@ build
 ALTER TABLE defaultdb.foo ADD COLUMN j INT
 ----
 - [[Column:{DescID: 104, ColumnID: 2}, PUBLIC], ABSENT]
-  {columnId: 2, pgAttributeNum: 2, tableId: 104}
+  {columnId: 2, tableId: 104}
 - [[ColumnName:{DescID: 104, Name: j, ColumnID: 2}, PUBLIC], ABSENT]
   {columnId: 2, name: j, tableId: 104}
 - [[ColumnType:{DescID: 104, ColumnFamilyID: 0, ColumnID: 2}, PUBLIC], ABSENT]
@@ -26,7 +26,7 @@ ALTER TABLE defaultdb.foo ADD COLUMN j INT NOT NULL DEFAULT 123
 - [[IndexData:{DescID: 104, IndexID: 1}, ABSENT], PUBLIC]
   {indexId: 1, tableId: 104}
 - [[Column:{DescID: 104, ColumnID: 2}, PUBLIC], ABSENT]
-  {columnId: 2, pgAttributeNum: 2, tableId: 104}
+  {columnId: 2, tableId: 104}
 - [[ColumnName:{DescID: 104, Name: j, ColumnID: 2}, PUBLIC], ABSENT]
   {columnId: 2, name: j, tableId: 104}
 - [[ColumnType:{DescID: 104, ColumnFamilyID: 0, ColumnID: 2}, PUBLIC], ABSENT]
@@ -67,7 +67,7 @@ ALTER TABLE defaultdb.foo ADD COLUMN k INT DEFAULT 456;
 - [[IndexData:{DescID: 104, IndexID: 1}, ABSENT], PUBLIC]
   {indexId: 1, tableId: 104}
 - [[Column:{DescID: 104, ColumnID: 2}, PUBLIC], ABSENT]
-  {columnId: 2, pgAttributeNum: 2, tableId: 104}
+  {columnId: 2, tableId: 104}
 - [[ColumnName:{DescID: 104, Name: j, ColumnID: 2}, PUBLIC], ABSENT]
   {columnId: 2, name: j, tableId: 104}
 - [[ColumnType:{DescID: 104, ColumnFamilyID: 0, ColumnID: 2}, PUBLIC], ABSENT]
@@ -93,7 +93,7 @@ ALTER TABLE defaultdb.foo ADD COLUMN k INT DEFAULT 456;
 - [[IndexData:{DescID: 104, IndexID: 3}, TRANSIENT_ABSENT], ABSENT]
   {indexId: 3, tableId: 104}
 - [[Column:{DescID: 104, ColumnID: 3}, PUBLIC], ABSENT]
-  {columnId: 3, pgAttributeNum: 3, tableId: 104}
+  {columnId: 3, tableId: 104}
 - [[ColumnName:{DescID: 104, Name: k, ColumnID: 3}, PUBLIC], ABSENT]
   {columnId: 3, name: k, tableId: 104}
 - [[ColumnType:{DescID: 104, ColumnFamilyID: 0, ColumnID: 3}, PUBLIC], ABSENT]
@@ -117,7 +117,7 @@ ALTER TABLE defaultdb.foo ADD COLUMN a INT AS (i+1) STORED
 - [[IndexData:{DescID: 104, IndexID: 1}, ABSENT], PUBLIC]
   {indexId: 1, tableId: 104}
 - [[Column:{DescID: 104, ColumnID: 2}, PUBLIC], ABSENT]
-  {columnId: 2, pgAttributeNum: 2, tableId: 104}
+  {columnId: 2, tableId: 104}
 - [[ColumnName:{DescID: 104, Name: a, ColumnID: 2}, PUBLIC], ABSENT]
   {columnId: 2, name: a, tableId: 104}
 - [[ColumnType:{DescID: 104, ColumnFamilyID: 0, ColumnID: 2}, PUBLIC], ABSENT]
@@ -150,15 +150,15 @@ ALTER TABLE defaultdb.foo ADD COLUMN a INT;
 ALTER TABLE defaultdb.bar ADD COLUMN b INT;
 ----
 - [[Column:{DescID: 104, ColumnID: 2}, PUBLIC], ABSENT]
-  {columnId: 2, pgAttributeNum: 2, tableId: 104}
+  {columnId: 2, tableId: 104}
 - [[ColumnName:{DescID: 104, Name: a, ColumnID: 2}, PUBLIC], ABSENT]
   {columnId: 2, name: a, tableId: 104}
 - [[ColumnType:{DescID: 104, ColumnFamilyID: 0, ColumnID: 2}, PUBLIC], ABSENT]
   {columnId: 2, elementCreationMetadata: {in231OrLater: true}, isNullable: true, tableId: 104, type: {family: IntFamily, oid: 20, width: 64}}
 - [[IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 1}, PUBLIC], ABSENT]
   {columnId: 2, indexId: 1, kind: STORED, tableId: 104}
 - [[Column:{DescID: 105, ColumnID: 3}, PUBLIC], ABSENT]
-  {columnId: 3, pgAttributeNum: 3, tableId: 105}
+  {columnId: 3, tableId: 105}
 - [[ColumnName:{DescID: 105, Name: b, ColumnID: 3}, PUBLIC], ABSENT]
   {columnId: 3, name: b, tableId: 105}
 - [[ColumnType:{DescID: 105, ColumnFamilyID: 0, ColumnID: 3}, PUBLIC], ABSENT]

diff --git a/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_primary_key b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_primary_key
@@ -7,7 +7,7 @@ build
 ALTER TABLE defaultdb.bar ADD PRIMARY KEY (i)
 ----
 - [[Column:{DescID: 105, ColumnID: 2}, ABSENT], PUBLIC]
-  {columnId: 2, isHidden: true, pgAttributeNum: 2, tableId: 105}
+  {columnId: 2, isHidden: true, tableId: 105}
 - [[ColumnName:{DescID: 105, Name: rowid, ColumnID: 2}, ABSENT], PUBLIC]
   {columnId: 2, name: rowid, tableId: 105}
 - [[ColumnType:{DescID: 105, ColumnFamilyID: 0, ColumnID: 2}, ABSENT], PUBLIC]

diff --git a/pkg/sql/schemachanger/scbuild/testdata/alter_table_alter_primary_key b/pkg/sql/schemachanger/scbuild/testdata/alter_table_alter_primary_key
@@ -67,7 +67,7 @@ build
 ALTER TABLE defaultdb.bar ALTER PRIMARY KEY USING COLUMNS (i)
 ----
 - [[Column:{DescID: 105, ColumnID: 2}, ABSENT], PUBLIC]
-  {columnId: 2, isHidden: true, pgAttributeNum: 2, tableId: 105}
+  {columnId: 2, isHidden: true, tableId: 105}
 - [[ColumnName:{DescID: 105, Name: rowid, ColumnID: 2}, ABSENT], PUBLIC]
   {columnId: 2, name: rowid, tableId: 105}
 - [[ColumnType:{DescID: 105, ColumnFamilyID: 0, ColumnID: 2}, ABSENT], PUBLIC]

diff --git a/pkg/sql/schemachanger/scbuild/testdata/alter_table_drop_column b/pkg/sql/schemachanger/scbuild/testdata/alter_table_drop_column
@@ -15,7 +15,7 @@ build
 ALTER TABLE defaultdb.t DROP COLUMN j
 ----
 - [[Column:{DescID: 104, ColumnID: 2}, ABSENT], PUBLIC]
-  {columnId: 2, pgAttributeNum: 2, tableId: 104}
+  {columnId: 2, tableId: 104}
 - [[ColumnName:{DescID: 104, Name: j, ColumnID: 2}, ABSENT], PUBLIC]
   {columnId: 2, name: j, tableId: 104}
 - [[ColumnType:{DescID: 104, ColumnFamilyID: 0, ColumnID: 2}, ABSENT], PUBLIC]
@@ -89,15 +89,15 @@ build
 ALTER TABLE defaultdb.t DROP COLUMN k, DROP COLUMN l
 ----
 - [[Column:{DescID: 104, ColumnID: 3}, ABSENT], PUBLIC]
-  {columnId: 3, pgAttributeNum: 3, tableId: 104}
+  {columnId: 3, tableId: 104}
 - [[ColumnName:{DescID: 104, Name: k, ColumnID: 3}, ABSENT], PUBLIC]
   {columnId: 3, name: k, tableId: 104}
 - [[ColumnType:{DescID: 104, ColumnFamilyID: 0, ColumnID: 3}, ABSENT], PUBLIC]
   {columnId: 3, elementCreationMetadata: {in231OrLater: true}, tableId: 104, type: {family: IntFamily, oid: 20, width: 64}}
 - [[ColumnNotNull:{DescID: 104, ColumnID: 3, IndexID: 0}, ABSENT], PUBLIC]
   {columnId: 3, tableId: 104}
 - [[Column:{DescID: 104, ColumnID: 4}, ABSENT], PUBLIC]
-  {columnId: 4, pgAttributeNum: 4, tableId: 104}
+  {columnId: 4, tableId: 104}
 - [[ColumnName:{DescID: 104, Name: l, ColumnID: 4}, ABSENT], PUBLIC]
   {columnId: 4, name: l, tableId: 104}
 - [[ColumnType:{DescID: 104, ColumnFamilyID: 0, ColumnID: 4}, ABSENT], PUBLIC]
@@ -171,7 +171,7 @@ build
 ALTER TABLE defaultdb.t DROP COLUMN l
 ----
 - [[Column:{DescID: 104, ColumnID: 4}, ABSENT], PUBLIC]
-  {columnId: 4, pgAttributeNum: 4, tableId: 104}
+  {columnId: 4, tableId: 104}
 - [[ColumnName:{DescID: 104, Name: l, ColumnID: 4}, ABSENT], PUBLIC]
   {columnId: 4, name: l, tableId: 104}
 - [[ColumnType:{DescID: 104, ColumnFamilyID: 0, ColumnID: 4}, ABSENT], PUBLIC]

diff --git a/pkg/sql/schemachanger/scbuild/testdata/comment_on b/pkg/sql/schemachanger/scbuild/testdata/comment_on
@@ -86,7 +86,7 @@ build
 COMMENT ON COLUMN customer.name IS 'some comment for name';
 ----
 - [[ColumnComment:{DescID: 104, ColumnID: 2, Comment: some comment for name}, PUBLIC], ABSENT]
-  {columnId: 2, comment: some comment for name, pgAttributeNum: 2, tableId: 104}
+  {columnId: 2, comment: some comment for name, tableId: 104}
 
 setup
 COMMENT ON COLUMN customer.name IS 'real comment for name';
@@ -96,7 +96,7 @@ build
 COMMENT ON COLUMN customer.name IS 'some better comment for name';
 ----
 - [[ColumnComment:{DescID: 104, ColumnID: 2, Comment: some better comment for name}, PUBLIC], ABSENT]
-  {columnId: 2, comment: some better comment for name, pgAttributeNum: 2, tableId: 104}
+  {columnId: 2, comment: some better comment for name, tableId: 104}
 
 build
 COMMENT ON COLUMN customer.name IS NULL;