Skip to content

Commit

Permalink
upgrades,sql: alter system.comments table primary key
Browse files Browse the repository at this point in the history 
Part of #88571

We need to read and write `system.comments` through `kv.Batch`
if we want extend the `uncommitted` layer of collection to cache
original comments and changes to comments, and be able to write
changes to comments together with other mutations to descriptor
in same transaction. To achieve that it'd be more convenient to
have the descriptor id as the first column of primary key, so that
we can construct a kv key like `/commentsTableID/descID` to fetch
all comments of an object. Otherwise we'd need to loop through
all comment types.

We're lucky that none of the existing code really rely on the old
primary key column order.

Release note: None
  • Loading branch information
@chengxiong-ruan
chengxiong-ruan committed Nov 1, 2022
1 parent 822f37f commit 95baec0
Show file tree
Hide file tree
Showing 13 changed files with 256 additions and 28 deletions.
2 changes: 1 addition & 1 deletion docs/generated/settings/settings-for-tenants.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,4 +298,4 @@ trace.jaeger.agent string the address of a Jaeger agent to receive traces using
trace.opentelemetry.collector string address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as <host>:<port>. If no port is specified, 4317 will be used.
trace.span_registry.enabled boolean true if set, ongoing traces can be seen at https://<ui>/#/debug/tracez
trace.zipkin.collector string the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.
version version 1000022.2-4 set the active cluster version in the format '<major>.<minor>'
version version 1000022.2-6 set the active cluster version in the format '<major>.<minor>'
2 changes: 1 addition & 1 deletion docs/generated/settings/settings.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,6 @@
<tr><td><code>trace.opentelemetry.collector</code></td><td>string</td><td><code></code></td><td>address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as <host>:<port>. If no port is specified, 4317 will be used.</td></tr>
<tr><td><code>trace.span_registry.enabled</code></td><td>boolean</td><td><code>true</code></td><td>if set, ongoing traces can be seen at https://<ui>/#/debug/tracez</td></tr>
<tr><td><code>trace.zipkin.collector</code></td><td>string</td><td><code></code></td><td>the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.</td></tr>
<tr><td><code>version</code></td><td>version</td><td><code>1000022.2-4</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
<tr><td><code>version</code></td><td>version</td><td><code>1000022.2-6</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
</tbody>
</table>
7 changes: 7 additions & 0 deletions pkg/clusterversion/cockroach_versions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,9 @@ const (
// TenantNames adds a name column to system.tenants.
V23_1TenantNames

// system.comments table uses (object_id, type, sub_id) as the new primary key.
V23_1CommentsSystemTableUseNewPrimaryKey

// *************************************************
// Step (1): Add new versions here.
// Do not add new versions to a patch release.
Expand Down Expand Up @@ -520,6 +523,10 @@ var rawVersionsSingleton = keyedVersions{
Key: V23_1TenantNames,
Version: roachpb.Version{Major: 22, Minor: 2, Internal: 4},
},
{
Key: V23_1CommentsSystemTableUseNewPrimaryKey,
Version: roachpb.Version{Major: 22, Minor: 2, Internal: 6},
},

// *************************************************
// Step (2): Add new versions here.
Expand Down
5 changes: 3 additions & 2 deletions pkg/clusterversion/key_string.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions pkg/sql/catalog/systemschema/system.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ CREATE TABLE system.comments (
object_id INT NOT NULL, -- object ID, this will be usually db/table desc ID
sub_id INT NOT NULL, -- sub ID for column or indexes inside table, 0 for pure table
comment STRING NOT NULL, -- the comment
CONSTRAINT "primary" PRIMARY KEY (type, object_id, sub_id)
CONSTRAINT "primary" PRIMARY KEY (object_id, type, sub_id)
);`

// reports_meta stores reports metadata
Expand Down Expand Up @@ -1554,9 +1554,9 @@ var (
Name: "primary",
ID: 1,
Unique: true,
KeyColumnNames: []string{"type", "object_id", "sub_id"},
KeyColumnNames: []string{"object_id", "type", "sub_id"},
KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC, catpb.IndexColumn_ASC, catpb.IndexColumn_ASC},
KeyColumnIDs: []descpb.ColumnID{1, 2, 3},
KeyColumnIDs: []descpb.ColumnID{2, 1, 3},
},
),
func(tbl *descpb.TableDescriptor) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/catalog/systemschema_test/testdata/bootstrap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ CREATE TABLE public.comments (
object_id INT8 NOT NULL,
sub_id INT8 NOT NULL,
comment STRING NOT NULL,
CONSTRAINT "primary" PRIMARY KEY (type ASC, object_id ASC, sub_id ASC),
CONSTRAINT "primary" PRIMARY KEY (object_id ASC, type ASC, sub_id ASC),
FAMILY "primary" (type, object_id, sub_id),
FAMILY fam_4_comment (comment)
);
Expand Down Expand Up @@ -396,7 +396,7 @@ schema_telemetry
{"database":{"name":"defaultdb","id":100,"modificationTime":{"wallTime":"0"},"version":"1","privileges":{"users":[{"userProto":"admin","privileges":2,"withGrantOption":2},{"userProto":"public","privileges":2048},{"userProto":"root","privileges":2,"withGrantOption":2}],"ownerProto":"root","version":2},"schemas":{"public":{"id":101}},"defaultPrivileges":{}}}
{"database":{"name":"postgres","id":102,"modificationTime":{"wallTime":"0"},"version":"1","privileges":{"users":[{"userProto":"admin","privileges":2,"withGrantOption":2},{"userProto":"public","privileges":2048},{"userProto":"root","privileges":2,"withGrantOption":2}],"ownerProto":"root","version":2},"schemas":{"public":{"id":103}},"defaultPrivileges":{}}}
{"database":{"name":"system","id":1,"modificationTime":{"wallTime":"0"},"version":"1","privileges":{"users":[{"userProto":"admin","privileges":2048,"withGrantOption":2048},{"userProto":"root","privileges":2048,"withGrantOption":2048}],"ownerProto":"node","version":2}}}
{"table":{"name":"comments","id":24,"version":"1","modificationTime":{"wallTime":"0"},"parentId":1,"unexposedParentSchemaId":29,"columns":[{"name":"type","id":1,"type":{"family":"IntFamily","width":64,"oid":20}},{"name":"object_id","id":2,"type":{"family":"IntFamily","width":64,"oid":20}},{"name":"sub_id","id":3,"type":{"family":"IntFamily","width":64,"oid":20}},{"name":"comment","id":4,"type":{"family":"StringFamily","oid":25}}],"nextColumnId":5,"families":[{"name":"primary","columnNames":["type","object_id","sub_id"],"columnIds":[1,2,3]},{"name":"fam_4_comment","id":4,"columnNames":["comment"],"columnIds":[4],"defaultColumnId":4}],"nextFamilyId":5,"primaryIndex":{"name":"primary","id":1,"unique":true,"version":4,"keyColumnNames":["type","object_id","sub_id"],"keyColumnDirections":["ASC","ASC","ASC"],"storeColumnNames":["comment"],"keyColumnIds":[1,2,3],"storeColumnIds":[4],"foreignKey":{},"interleave":{},"partitioning":{},"encodingType":1,"sharded":{},"geoConfig":{},"constraintId":1},"nextIndexId":2,"privileges":{"users":[{"userProto":"admin","privileges":480,"withGrantOption":480},{"userProto":"public","privileges":32},{"userProto":"root","privileges":480,"withGrantOption":480}],"ownerProto":"node","version":2},"nextMutationId":1,"formatVersion":3,"replacementOf":{"time":{}},"createAsOfTime":{"wallTime":"0"},"nextConstraintId":2}}
{"table":{"name":"comments","id":24,"version":"1","modificationTime":{"wallTime":"0"},"parentId":1,"unexposedParentSchemaId":29,"columns":[{"name":"type","id":1,"type":{"family":"IntFamily","width":64,"oid":20}},{"name":"object_id","id":2,"type":{"family":"IntFamily","width":64,"oid":20}},{"name":"sub_id","id":3,"type":{"family":"IntFamily","width":64,"oid":20}},{"name":"comment","id":4,"type":{"family":"StringFamily","oid":25}}],"nextColumnId":5,"families":[{"name":"primary","columnNames":["type","object_id","sub_id"],"columnIds":[1,2,3]},{"name":"fam_4_comment","id":4,"columnNames":["comment"],"columnIds":[4],"defaultColumnId":4}],"nextFamilyId":5,"primaryIndex":{"name":"primary","id":1,"unique":true,"version":4,"keyColumnNames":["object_id","type","sub_id"],"keyColumnDirections":["ASC","ASC","ASC"],"storeColumnNames":["comment"],"keyColumnIds":[2,1,3],"storeColumnIds":[4],"foreignKey":{},"interleave":{},"partitioning":{},"encodingType":1,"sharded":{},"geoConfig":{},"constraintId":1},"nextIndexId":2,"privileges":{"users":[{"userProto":"admin","privileges":480,"withGrantOption":480},{"userProto":"public","privileges":32},{"userProto":"root","privileges":480,"withGrantOption":480}],"ownerProto":"node","version":2},"nextMutationId":1,"formatVersion":3,"replacementOf":{"time":{}},"createAsOfTime":{"wallTime":"0"},"nextConstraintId":2}}
{"table":{"name":"database_role_settings","id":44,"version":"1","modificationTime":{"wallTime":"0"},"parentId":1,"unexposedParentSchemaId":29,"columns":[{"name":"database_id","id":1,"type":{"family":"OidFamily","oid":26}},{"name":"role_name","id":2,"type":{"family":"StringFamily","oid":25}},{"name":"settings","id":3,"type":{"family":"ArrayFamily","arrayElemType":"StringFamily","oid":1009,"arrayContents":{"family":"StringFamily","oid":25}}}],"nextColumnId":4,"families":[{"name":"primary","columnNames":["database_id","role_name","settings"],"columnIds":[1,2,3],"defaultColumnId":3}],"nextFamilyId":1,"primaryIndex":{"name":"primary","id":1,"unique":true,"version":4,"keyColumnNames":["database_id","role_name"],"keyColumnDirections":["ASC","ASC"],"storeColumnNames":["settings"],"keyColumnIds":[1,2],"storeColumnIds":[3],"foreignKey":{},"interleave":{},"partitioning":{},"encodingType":1,"sharded":{},"geoConfig":{},"constraintId":1},"nextIndexId":2,"privileges":{"users":[{"userProto":"admin","privileges":480,"withGrantOption":480},{"userProto":"root","privileges":480,"withGrantOption":480}],"ownerProto":"node","version":2},"nextMutationId":1,"formatVersion":3,"replacementOf":{"time":{}},"createAsOfTime":{"wallTime":"0"},"nextConstraintId":2}}
{"table":{"name":"descriptor","id":3,"version":"1","modificationTime":{"wallTime":"0"},"parentId":1,"unexposedParentSchemaId":29,"columns":[{"name":"id","id":1,"type":{"family":"IntFamily","width":64,"oid":20}},{"name":"descriptor","id":2,"type":{"family":"BytesFamily","oid":17},"nullable":true}],"nextColumnId":3,"families":[{"name":"primary","columnNames":["id"],"columnIds":[1]},{"name":"fam_2_descriptor","id":2,"columnNames":["descriptor"],"columnIds":[2],"defaultColumnId":2}],"nextFamilyId":3,"primaryIndex":{"name":"primary","id":1,"unique":true,"version":4,"keyColumnNames":["id"],"keyColumnDirections":["ASC"],"storeColumnNames":["descriptor"],"keyColumnIds":[1],"storeColumnIds":[2],"foreignKey":{},"interleave":{},"partitioning":{},"encodingType":1,"sharded":{},"geoConfig":{},"constraintId":1},"nextIndexId":2,"privileges":{"users":[{"userProto":"admin","privileges":32,"withGrantOption":32},{"userProto":"root","privileges":32,"withGrantOption":32}],"ownerProto":"node","version":2},"nextMutationId":1,"formatVersion":3,"replacementOf":{"time":{}},"createAsOfTime":{"wallTime":"0"},"nextConstraintId":2}}
{"table":{"name":"eventlog","id":12,"version":"1","modificationTime":{"wallTime":"0"},"parentId":1,"unexposedParentSchemaId":29,"columns":[{"name":"timestamp","id":1,"type":{"family":"TimestampFamily","oid":1114}},{"name":"eventType","id":2,"type":{"family":"StringFamily","oid":25}},{"name":"targetID","id":3,"type":{"family":"IntFamily","width":64,"oid":20}},{"name":"reportingID","id":4,"type":{"family":"IntFamily","width":64,"oid":20}},{"name":"info","id":5,"type":{"family":"StringFamily","oid":25},"nullable":true},{"name":"uniqueID","id":6,"type":{"family":"BytesFamily","oid":17},"defaultExpr":"uuid_v4()"}],"nextColumnId":7,"families":[{"name":"primary","columnNames":["timestamp","uniqueID"],"columnIds":[1,6]},{"name":"fam_2_eventType","id":2,"columnNames":["eventType"],"columnIds":[2],"defaultColumnId":2},{"name":"fam_3_targetID","id":3,"columnNames":["targetID"],"columnIds":[3],"defaultColumnId":3},{"name":"fam_4_reportingID","id":4,"columnNames":["reportingID"],"columnIds":[4],"defaultColumnId":4},{"name":"fam_5_info","id":5,"columnNames":["info"],"columnIds":[5],"defaultColumnId":5}],"nextFamilyId":6,"primaryIndex":{"name":"primary","id":1,"unique":true,"version":4,"keyColumnNames":["timestamp","uniqueID"],"keyColumnDirections":["ASC","ASC"],"storeColumnNames":["eventType","targetID","reportingID","info"],"keyColumnIds":[1,6],"storeColumnIds":[2,3,4,5],"foreignKey":{},"interleave":{},"partitioning":{},"encodingType":1,"sharded":{},"geoConfig":{},"constraintId":1},"nextIndexId":2,"privileges":{"users":[{"userProto":"admin","privileges":480,"withGrantOption":480},{"userProto":"root","privileges":480,"withGrantOption":480}],"ownerProto":"node","version":2},"nextMutationId":1,"formatVersion":3,"replacementOf":{"time":{}},"createAsOfTime":{"wallTime":"0"},"nextConstraintId":2}}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/pg_catalog
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1141,7 +1141,7 @@ indexrelid indrelid indnatts indisunique indisprimary indisexclusion indim
2268653844 40 4 true true false true false true false false true false 1 2 3 4 0 0 0 0 0 0 0 0 2 2 2 2 NULL NULL 4
2361445172 8 1 true true false true false true false false true false 1 0 0 2 NULL NULL 1
2361445175 8 1 true false false true false true false false true false 4 3403232968 0 2 NULL NULL 1
2407840836 24 3 true true false true false true false false true false 1 2 3 0 0 0 0 0 0 2 2 2 NULL NULL 3
2407840836 24 3 true true false true false true false false true false 2 1 3 0 0 0 0 0 0 2 2 2 NULL NULL 3
2528390115 47 1 true true false true false true false false true false 1 0 0 2 NULL NULL 1
2621181440 15 2 false false false false false true false false true false 2 3 3403232968 0 0 0 2 2 NULL NULL 2
2621181441 15 3 false false false false false true false false true false 6 7 2 3403232968 0 0 0 2 2 NULL NULL 2
Expand Down
2 changes: 2 additions & 0 deletions pkg/upgrade/upgrades/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ go_library(
srcs = [
"alter_sql_instances_locality.go",
"alter_statement_statistics_index_recommendations.go",
"alter_system_comments_primary_key.go",
"descriptor_utils.go",
"ensure_sql_schema_telemetry_schedule.go",
"fix_userfile_descriptor_corruption.go",
Expand Down Expand Up @@ -72,6 +73,7 @@ go_test(
srcs = [
"alter_sql_instances_locality_test.go",
"alter_statement_statistics_index_recommendations_test.go",
"alter_system_comments_primary_key_test.go",
"builtins_test.go",
"descriptor_utils_test.go",
"ensure_sql_schema_telemetry_schedule_test.go",
Expand Down
Loading

0 comments on commit 95baec0

Please sign in to comment.