sql: use SQL sequence as DescIDGenerator for non-system tenants

Fixes cockroachdb#48513.

This commit creates a new SQL sequence called "descriptor_id_seq" that
is used by all non-system tenants to allocate descriptor IDs. This is
necessary because non-system tenants should not be using the global
`DescIDGenerator` key to allocate IDs.

The next commit includes tests that exercise the MetadataSchema portion
of this change.

pkg/ccl/backupccl/backup_test.go

@@ -1103,7 +1103,7 @@ func TestBackupRestoreResume(t *testing.T) {
 		sqlDB.Exec(t, `BACKUP DATABASE DATA TO $1`, restoreDir)
 		sqlDB.Exec(t, `CREATE DATABASE restoredb`)
 		restoreDatabaseID := sqlutils.QueryDatabaseID(t, sqlDB.DB, "restoredb")
-		restoreTableID, err := sql.GenerateUniqueDescID(ctx, tc.Servers[0].DB())
+		restoreTableID, err := sql.GenerateUniqueDescID(ctx, tc.Servers[0].DB(), keys.SystemSQLCodec)
 		if err != nil {
 			t.Fatal(err)
 		}

pkg/ccl/backupccl/restore_planning.go

@@ -204,14 +204,14 @@ func allocateTableRewrites(
 		//   since for clusters with many descrirptors we'd want to avoid
 		//   incrementing it 10,000+ times.
 		for i := uint32(0); i <= numberOfIncrements; i++ {
-			_, err = sql.GenerateUniqueDescID(ctx, p.ExecCfg().DB)
+			_, err = sql.GenerateUniqueDescID(ctx, p.ExecCfg().DB, p.ExecCfg().Codec)
 			if err != nil {
 				return nil, err
 			}
 		}
 
 		// Generate one more desc ID for the ID of the temporary system db.
-		tempSysDBID, err = sql.GenerateUniqueDescID(ctx, p.ExecCfg().DB)
+		tempSysDBID, err = sql.GenerateUniqueDescID(ctx, p.ExecCfg().DB, p.ExecCfg().Codec)
 		if err != nil {
 			return nil, err
 		}
@@ -338,7 +338,7 @@ func allocateTableRewrites(
 		if descriptorCoverage == tree.AllDescriptors {
 			newID = db.ID
 		} else {
-			newID, err = sql.GenerateUniqueDescID(ctx, p.ExecCfg().DB)
+			newID, err = sql.GenerateUniqueDescID(ctx, p.ExecCfg().DB, p.ExecCfg().Codec)
 			if err != nil {
 				return nil, err
 			}
@@ -372,7 +372,7 @@ func allocateTableRewrites(
 
 	// Generate new IDs for the tables that need to be remapped.
 	for _, table := range tablesToRemap {
-		newTableID, err := sql.GenerateUniqueDescID(ctx, p.ExecCfg().DB)
+		newTableID, err := sql.GenerateUniqueDescID(ctx, p.ExecCfg().DB, p.ExecCfg().Codec)
 		if err != nil {
 			return nil, err
 		}

pkg/ccl/importccl/import_stmt.go

@@ -871,7 +871,7 @@ func prepareNewTableDescsForIngestion(
 	tableRewrites := make(backupccl.TableRewriteMap)
 	seqVals := make(map[sqlbase.ID]int64, len(tables))
 	for _, tableDesc := range tables {
-		id, err := sql.GenerateUniqueDescID(ctx, p.ExecCfg().DB)
+		id, err := sql.GenerateUniqueDescID(ctx, p.ExecCfg().DB, p.ExecCfg().Codec)
 		if err != nil {
 			return nil, err
 		}

pkg/keys/constants.go

@@ -332,6 +332,7 @@ const (
 	UsersTableID               = 4
 	ZonesTableID               = 5
 	SettingsTableID            = 6
+	DescIDSequenceID           = 7
 
 	// IDs for the important columns and indexes in the zones table live here to
 	// avoid introducing a dependency on sql/sqlbase throughout the codebase.

pkg/keys/sql.go

@@ -87,6 +87,13 @@ var SystemSQLCodec = MakeSQLCodec(roachpb.SystemTenantID)
 // surrounding context.
 var TODOSQLCodec = MakeSQLCodec(roachpb.SystemTenantID)
 
+// systemTenant returns whether the encoder is bound to the system tenant. This
+// information should not be exposed, but is used internally to the encoder when
+// key encoding for the system tenant differs from that of all other tenants.
+func (e sqlEncoder) systemTenant() bool {
+	return len(e.TenantPrefix()) == 0
+}
+
 // TenantPrefix returns the key prefix used for the tenants's data.
 func (e sqlEncoder) TenantPrefix() roachpb.Key {
 	return *e.buf
@@ -124,6 +131,16 @@ func (e sqlEncoder) SequenceKey(tableID uint32) roachpb.Key {
 	return k
 }
 
+// DescIDSequenceKey returns the key used for the descriptor ID sequence.
+func (e sqlEncoder) DescIDSequenceKey() roachpb.Key {
+	if e.systemTenant() {
+		// To maintain backwards compatibility, the system tenant uses a
+		// separate, non-SQL, key to store its descriptor ID sequence.
+		return DescIDGenerator
+	}
+	return e.SequenceKey(DescIDSequenceID)
+}
+
 // ZoneKeyPrefix returns the key prefix for id's row in the system.zones table.
 func (e sqlEncoder) ZoneKeyPrefix(id uint32) roachpb.Key {
 	k := e.IndexPrefix(ZonesTableID, ZonesTablePrimaryIndexID)

pkg/sql/create_sequence.go

@@ -82,7 +82,7 @@ func doCreateSequence(
 	opts tree.SequenceOptions,
 	jobDesc string,
 ) error {
-	id, err := GenerateUniqueDescID(params.ctx, params.p.ExecCfg().DB)
+	id, err := GenerateUniqueDescID(params.ctx, params.p.ExecCfg().DB, params.p.ExecCfg().Codec)
 	if err != nil {
 		return err
 	}
@@ -157,11 +157,6 @@ func (*createSequenceNode) Next(runParams) (bool, error) { return false, nil }
 func (*createSequenceNode) Values() tree.Datums          { return tree.Datums{} }
 func (*createSequenceNode) Close(context.Context)        {}
 
-const (
-	sequenceColumnID   = 1
-	sequenceColumnName = "value"
-)
-
 // MakeSequenceTableDesc creates a sequence descriptor.
 func MakeSequenceTableDesc(
 	sequenceName string,
@@ -187,25 +182,25 @@ func MakeSequenceTableDesc(
 	// Mimic a table with one column, "value".
 	desc.Columns = []sqlbase.ColumnDescriptor{
 		{
-			ID:   1,
-			Name: sequenceColumnName,
+			ID:   sqlbase.SequenceColumnID,
+			Name: sqlbase.SequenceColumnName,
 			Type: types.Int,
 		},
 	}
 	desc.PrimaryIndex = sqlbase.IndexDescriptor{
 		ID:               keys.SequenceIndexID,
 		Name:             sqlbase.PrimaryKeyIndexName,
-		ColumnIDs:        []sqlbase.ColumnID{sqlbase.ColumnID(1)},
-		ColumnNames:      []string{sequenceColumnName},
+		ColumnIDs:        []sqlbase.ColumnID{sqlbase.SequenceColumnID},
+		ColumnNames:      []string{sqlbase.SequenceColumnName},
 		ColumnDirections: []sqlbase.IndexDescriptor_Direction{sqlbase.IndexDescriptor_ASC},
 	}
 	desc.Families = []sqlbase.ColumnFamilyDescriptor{
 		{
 			ID:              keys.SequenceColumnFamilyID,
-			ColumnIDs:       []sqlbase.ColumnID{1},
-			ColumnNames:     []string{sequenceColumnName},
+			ColumnIDs:       []sqlbase.ColumnID{sqlbase.SequenceColumnID},
+			ColumnNames:     []string{sqlbase.SequenceColumnName},
 			Name:            "primary",
-			DefaultColumnID: sequenceColumnID,
+			DefaultColumnID: sqlbase.SequenceColumnID,
 		},
 	}
 

pkg/sql/create_table.go

@@ -261,7 +261,7 @@ func (n *createTableNode) startExec(params runParams) error {
 		}
 	}
 
-	id, err := GenerateUniqueDescID(params.ctx, params.extendedEvalCtx.ExecCfg.DB)
+	id, err := GenerateUniqueDescID(params.ctx, params.p.ExecCfg().DB, params.p.ExecCfg().Codec)
 	if err != nil {
 		return err
 	}

pkg/sql/create_type.go

@@ -91,7 +91,7 @@ func getCreateTypeParams(
 	if err != nil {
 		return nil, 0, err
 	}
-	id, err := GenerateUniqueDescID(params.ctx, params.extendedEvalCtx.ExecCfg.DB)
+	id, err := GenerateUniqueDescID(params.ctx, params.ExecCfg().DB, params.ExecCfg().Codec)
 	if err != nil {
 		return nil, 0, err
 	}

pkg/sql/create_view.go

@@ -128,7 +128,7 @@ func (n *createViewNode) startExec(params runParams) error {
 		}
 	} else {
 		// If we aren't replacing anything, make a new table descriptor.
-		id, err := GenerateUniqueDescID(params.ctx, params.extendedEvalCtx.ExecCfg.DB)
+		id, err := GenerateUniqueDescID(params.ctx, params.p.ExecCfg().DB, params.p.ExecCfg().Codec)
 		if err != nil {
 			return err
 		}

pkg/sql/descriptor.go

@@ -55,9 +55,9 @@ var MaxDefaultDescriptorID = keys.MaxReservedDescID + sqlbase.ID(len(DefaultUser
 // GenerateUniqueDescID returns the next available Descriptor ID and increments
 // the counter. The incrementing is non-transactional, and the counter could be
 // incremented multiple times because of retries.
-func GenerateUniqueDescID(ctx context.Context, db *kv.DB) (sqlbase.ID, error) {
+func GenerateUniqueDescID(ctx context.Context, db *kv.DB, codec keys.SQLCodec) (sqlbase.ID, error) {
 	// Increment unique descriptor counter.
-	newVal, err := kv.IncrementValRetryable(ctx, db, keys.DescIDGenerator, 1)
+	newVal, err := kv.IncrementValRetryable(ctx, db, codec.DescIDSequenceKey(), 1)
 	if err != nil {
 		return sqlbase.InvalidID, err
 	}
@@ -92,7 +92,7 @@ func (p *planner) createDatabase(
 		return false, err
 	}
 
-	id, err := GenerateUniqueDescID(ctx, p.ExecCfg().DB)
+	id, err := GenerateUniqueDescID(ctx, p.ExecCfg().DB, p.ExecCfg().Codec)
 	if err != nil {
 		return false, err
 	}

pkg/sql/sqlbase/metadata.go

@@ -95,6 +95,12 @@ func MakeMetadataSchema(
 	return ms
 }
 
+// forSystemTenant returns whether this MetadataSchema is associated with the
+// system tenant or with a secondary tenant.
+func (ms *MetadataSchema) forSystemTenant() bool {
+	return ms.codec == keys.SystemSQLCodec
+}
+
 // AddDescriptor adds a new non-config descriptor to the system schema.
 func (ms *MetadataSchema) AddDescriptor(parentID ID, desc DescriptorProto) {
 	if id := desc.GetID(); id > keys.MaxReservedDescID {
@@ -137,7 +143,7 @@ func (ms MetadataSchema) GetInitialValues() ([]roachpb.KeyValue, []roachpb.RKey)
 	value := roachpb.Value{}
 	value.SetInt(int64(keys.MinUserDescID))
 	ret = append(ret, roachpb.KeyValue{
-		Key:   keys.DescIDGenerator,
+		Key:   ms.codec.DescIDSequenceKey(),
 		Value: value,
 	})
 

pkg/sql/sqlbase/structured.go

@@ -206,6 +206,10 @@ const InvalidMutationID MutationID = 0
 const (
 	// PrimaryKeyIndexName is the name of the index for the primary key.
 	PrimaryKeyIndexName = "primary"
+	// SequenceColumnID is the ID of the sole column in a sequence.
+	SequenceColumnID = 1
+	// SequenceColumnName is the name of the sole column in a sequence.
+	SequenceColumnName = "value"
 )
 
 // ErrMissingColumns indicates a table with no columns.

pkg/sql/sqlbase/system.go

@@ -12,6 +12,7 @@ package sqlbase
 
 import (
 	"fmt"
+	"math"
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/config/zonepb"
@@ -96,6 +97,9 @@ CREATE TABLE system.settings (
 	"valueType"       STRING,
 	FAMILY (name, value, "lastUpdated", "valueType")
 );`
+
+	DescIDSequenceSchema = `
+CREATE SEQUENCE system.descriptor_id_seq;`
 )
 
 // These system tables are not part of the system config.
@@ -290,7 +294,7 @@ create table system.statement_diagnostics(
 
 func pk(name string) IndexDescriptor {
 	return IndexDescriptor{
-		Name:             "primary",
+		Name:             PrimaryKeyIndexName,
 		ID:               1,
 		Unique:           true,
 		ColumnNames:      []string{name},
@@ -315,6 +319,7 @@ var SystemAllowedPrivileges = map[ID]privilege.List{
 	// the use of a validating, logging accessor, so we'll go ahead and tolerate
 	// read-only privs to make that migration possible later.
 	keys.SettingsTableID:   privilege.ReadWriteData,
+	keys.DescIDSequenceID:  privilege.ReadData,
 	keys.LeaseTableID:      privilege.ReadWriteData,
 	keys.EventLogTableID:   privilege.ReadWriteData,
 	keys.RangeEventTableID: privilege.ReadWriteData,
@@ -567,6 +572,40 @@ var (
 		FormatVersion:  InterleavedFormatVersion,
 		NextMutationID: 1,
 	}
+
+	// DescIDSequence is the descriptor for the descriptor ID sequence.
+	DescIDSequence = TableDescriptor{
+		Name:                    "descriptor_id_seq",
+		ID:                      keys.DescIDSequenceID,
+		ParentID:                keys.SystemDatabaseID,
+		UnexposedParentSchemaID: keys.PublicSchemaID,
+		Version:                 1,
+		Columns: []ColumnDescriptor{
+			{Name: SequenceColumnName, ID: SequenceColumnID, Type: types.Int},
+		},
+		Families: []ColumnFamilyDescriptor{{
+			Name:            "primary",
+			ID:              keys.SequenceColumnFamilyID,
+			ColumnNames:     []string{SequenceColumnName},
+			ColumnIDs:       []ColumnID{SequenceColumnID},
+			DefaultColumnID: SequenceColumnID,
+		}},
+		PrimaryIndex: IndexDescriptor{
+			ID:               keys.SequenceIndexID,
+			Name:             PrimaryKeyIndexName,
+			ColumnIDs:        []ColumnID{SequenceColumnID},
+			ColumnNames:      []string{SequenceColumnName},
+			ColumnDirections: []IndexDescriptor_Direction{IndexDescriptor_ASC},
+		},
+		SequenceOpts: &TableDescriptor_SequenceOpts{
+			Increment: 1,
+			MinValue:  1,
+			MaxValue:  math.MaxInt64,
+			Start:     1,
+		},
+		Privileges:    NewCustomSuperuserPrivilegeDescriptor(SystemAllowedPrivileges[keys.DescIDSequenceID]),
+		FormatVersion: InterleavedFormatVersion,
+	}
 )
 
 // These system TableDescriptor literals should match the descriptor that
@@ -1486,6 +1525,12 @@ func addSystemDescriptorsToSchema(target *MetadataSchema) {
 	target.AddDescriptor(keys.SystemDatabaseID, &ZonesTable)
 	target.AddDescriptor(keys.SystemDatabaseID, &SettingsTable)
 
+	// Only add the descriptor ID sequence if this is a non-system tenant.
+	// System tenants use the global DescIDGenerator key. See #48513.
+	if !target.forSystemTenant() {
+		target.AddDescriptor(keys.SystemDatabaseID, &DescIDSequence)
+	}
+
 	// Add all the other system tables.
 	target.AddDescriptor(keys.SystemDatabaseID, &LeaseTable)
 	target.AddDescriptor(keys.SystemDatabaseID, &EventLogTable)

pkg/sql/temporary_schema.go

@@ -83,7 +83,7 @@ var (
 )
 
 func createTempSchema(params runParams, sKey sqlbase.DescriptorKey) (sqlbase.ID, error) {
-	id, err := GenerateUniqueDescID(params.ctx, params.extendedEvalCtx.ExecCfg.DB)
+	id, err := GenerateUniqueDescID(params.ctx, params.p.ExecCfg().DB, params.p.ExecCfg().Codec)
 	if err != nil {
 		return sqlbase.InvalidID, err
 	}

pkg/sql/tests/system_table_test.go

@@ -112,6 +112,7 @@ func TestSystemTableLiterals(t *testing.T) {
 		{keys.UITableID, sqlbase.UITableSchema, sqlbase.UITable},
 		{keys.JobsTableID, sqlbase.JobsTableSchema, sqlbase.JobsTable},
 		{keys.SettingsTableID, sqlbase.SettingsTableSchema, sqlbase.SettingsTable},
+		{keys.DescIDSequenceID, sqlbase.DescIDSequenceSchema, sqlbase.DescIDSequence},
 		{keys.WebSessionsTableID, sqlbase.WebSessionsTableSchema, sqlbase.WebSessionsTable},
 		{keys.TableStatisticsTableID, sqlbase.TableStatisticsTableSchema, sqlbase.TableStatisticsTable},
 		{keys.LocationsTableID, sqlbase.LocationsTableSchema, sqlbase.LocationsTable},

pkg/sql/testutils.go

@@ -20,6 +20,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
+	"github.com/cockroachdb/errors"
 )
 
 // CreateTestTableDescriptor converts a SQL string to a table for test purposes.
@@ -38,22 +39,38 @@ func CreateTestTableDescriptor(
 	}
 	semaCtx := tree.MakeSemaContext()
 	evalCtx := tree.MakeTestingEvalContext(st)
-	desc, err := MakeTableDesc(
-		ctx,
-		nil, /* txn */
-		nil, /* vt */
-		st,
-		stmt.AST.(*tree.CreateTable),
-		parentID, keys.PublicSchemaID, id,
-		hlc.Timestamp{}, /* creationTime */
-		privileges,
-		nil, /* affected */
-		&semaCtx,
-		&evalCtx,
-		&sessiondata.SessionData{}, /* sessionData */
-		false,                      /* temporary */
-	)
-	return desc.TableDescriptor, err
+	switch n := stmt.AST.(type) {
+	case *tree.CreateTable:
+		desc, err := MakeTableDesc(
+			ctx,
+			nil, /* txn */
+			nil, /* vt */
+			st,
+			n,
+			parentID, keys.PublicSchemaID, id,
+			hlc.Timestamp{}, /* creationTime */
+			privileges,
+			nil, /* affected */
+			&semaCtx,
+			&evalCtx,
+			&sessiondata.SessionData{}, /* sessionData */
+			false,                      /* temporary */
+		)
+		return desc.TableDescriptor, err
+	case *tree.CreateSequence:
+		desc, err := MakeSequenceTableDesc(
+			n.Name.Table(),
+			n.Options,
+			parentID, keys.PublicSchemaID, id,
+			hlc.Timestamp{}, /* creationTime */
+			privileges,
+			false, /* temporary */
+			nil,   /* params */
+		)
+		return desc.TableDescriptor, err
+	default:
+		return sqlbase.TableDescriptor{}, errors.Errorf("unexpected AST %T", stmt.AST)
+	}
 }
 
 // StmtBufReader is an exported interface for reading a StmtBuf.