Merge #28575

28575: sql: introduce virtual sequences and use with SERIAL r=knz a=knz

Very good idea by @BramGruneir 

New features: virtual sequences (opt-in) and create-sequence-upon-SERIAL (opt-in). See individual commits for details and release notes.

For reference:

- I made everything opt-in here (i.e. default to previous CockroachDB behavior) to minimize risk.
- UX with Hibernate will thus not automatically be improved. We need to inform users (and upgrade our tutorials/docs) that this experimental feature is available and how to use it to unblock problems.
- It is possible that making the new default be `virtual_sequence` will both be backward-compatible with performance and unlock UX for Hibernate users. This has @awoods187 preference I think. However I propose to only flip that switch after we've seen a week or two pass with this patch merged.

Fixes #22607 (using the new setting set to `sql_sequence`).
Fixes #24062 (using the new setting set to `sql_sequence`).
Fixes #26730 (using the new setting set to either `virtual_sequence` or `sql_sequence`).



Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>

docs/generated/sql/bnf/alter_sequence_options_stmt.bnf

@@ -1,3 +1,3 @@
 alter_sequence_options_stmt ::=
-	'ALTER' 'SEQUENCE' sequence_name ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer ) ) ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer ) ) )* )
-	| 'ALTER' 'SEQUENCE' 'IF' 'EXISTS' sequence_name ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer ) ) ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer ) ) )* )
+	'ALTER' 'SEQUENCE' sequence_name ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'VIRTUAL' ) ) ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'VIRTUAL' ) ) )* )
+	| 'ALTER' 'SEQUENCE' 'IF' 'EXISTS' sequence_name ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'VIRTUAL' ) ) ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'VIRTUAL' ) ) )* )

docs/generated/sql/bnf/create_sequence_stmt.bnf

@@ -1,3 +1,3 @@
 create_sequence_stmt ::=
-	'CREATE' 'SEQUENCE' sequence_name ( ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer ) ) ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer ) ) )* ) |  )
-	| 'CREATE' 'SEQUENCE' 'IF' 'NOT' 'EXISTS' sequence_name ( ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer ) ) ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer ) ) )* ) |  )
+	'CREATE' 'SEQUENCE' sequence_name ( ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'VIRTUAL' ) ) ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'VIRTUAL' ) ) )* ) |  )
+	| 'CREATE' 'SEQUENCE' 'IF' 'NOT' 'EXISTS' sequence_name ( ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'VIRTUAL' ) ) ( ( ( 'NO' 'CYCLE' | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'VIRTUAL' ) ) )* ) |  )

docs/generated/sql/bnf/stmt_block.bnf

@@ -1768,6 +1768,7 @@ sequence_option_elem ::=
 	| 'NO' 'MAXVALUE'
 	| 'START' signed_iconst64
 	| 'START' 'WITH' signed_iconst64
+	| 'VIRTUAL'
 
 opt_asc_desc ::=
 	'ASC'

pkg/ccl/importccl/import_stmt.go

@@ -136,6 +136,10 @@ var NoFKs = fkHandler{}
 // node without the full machinery. Many parts of the syntax are unsupported
 // (see the implementation and TestMakeSimpleTableDescriptorErrors for details),
 // but this is enough for our csv IMPORT and for some unit tests.
+//
+// Any occurrence of SERIAL in the column definitions is handled using
+// the CockroachDB legacy behavior, i.e. INT NOT NULL DEFAULT
+// unique_rowid().
 func MakeSimpleTableDescriptor(
 	ctx context.Context,
 	st *cluster.Settings,
@@ -145,7 +149,6 @@ func MakeSimpleTableDescriptor(
 	fks fkHandler,
 	walltime int64,
 ) (*sqlbase.TableDescriptor, error) {
-
 	sql.HoistConstraints(create)
 	if create.IfNotExists {
 		return nil, errors.New("unsupported IF NOT EXISTS")
@@ -156,6 +159,12 @@ func MakeSimpleTableDescriptor(
 	if create.AsSource != nil {
 		return nil, errors.New("CREATE AS not supported")
 	}
+
+	tableName, err := create.Table.Normalize()
+	if err != nil {
+		return nil, err
+	}
+
 	filteredDefs := create.Defs[:0]
 	for i := range create.Defs {
 		switch def := create.Defs[i].(type) {
@@ -168,15 +177,26 @@ func MakeSimpleTableDescriptor(
 			if def.Computed.Expr != nil {
 				return nil, errors.Errorf("computed columns not supported: %s", tree.AsString(def))
 			}
+
+			if err := sql.SimplifySerialInColumnDefWithRowID(ctx, def, tableName); err != nil {
+				return nil, err
+			}
+
 		case *tree.ForeignKeyConstraintTableDef:
 			if !fks.allowed {
 				return nil, errors.Errorf("this IMPORT format does not support foreign keys")
 			}
 			if fks.skip {
 				continue
 			}
-			n := tree.MakeTableName("", tree.Name(def.Table.TableNameReference.String()))
-			def.Table.TableNameReference = &n
+			// Strip the schema/db prefix.
+			refTable, err := def.Table.Normalize()
+			if err != nil {
+				return nil, err
+			}
+			*refTable = tree.MakeUnqualifiedTableName(refTable.TableName)
+			def.Table.TableNameReference = refTable
+
 		default:
 			return nil, errors.Errorf("unsupported table definition: %s", tree.AsString(def))
 		}
@@ -191,6 +211,7 @@ func MakeSimpleTableDescriptor(
 		Sequence: &importSequenceOperators{},
 	}
 	affected := make(map[sqlbase.ID]*sqlbase.TableDescriptor)
+
 	tableDesc, err := sql.MakeTableDesc(
 		ctx,
 		nil, /* txn */

pkg/ccl/importccl/import_stmt_test.go

@@ -1274,29 +1274,43 @@ func TestImportCSVStmt(t *testing.T) {
 			nullif = ` WITH nullif=''`
 		)
 
-		data = ",5,e,,,"
-		if _, err := conn.Exec(query, srv.URL); !testutils.IsError(err, `row 1: parse "a" as INT: could not parse ""`) {
-			t.Fatalf("unexpected: %v", err)
-		}
-		if _, err := conn.Exec(query+nullif, srv.URL); !testutils.IsError(err, `row 1: generate insert row: null value in column "a" violates not-null constraint`) {
-			t.Fatalf("unexpected: %v", err)
-		}
+		data = ",5,e,7,,"
+		t.Run(data, func(t *testing.T) {
+			if _, err := conn.Exec(query, srv.URL); !testutils.IsError(err, `row 1: parse "a" as INT: could not parse ""`) {
+				t.Fatalf("unexpected: %v", err)
+			}
+			if _, err := conn.Exec(query+nullif, srv.URL); !testutils.IsError(err, `row 1: generate insert row: null value in column "a" violates not-null constraint`) {
+				t.Fatalf("unexpected: %v", err)
+			}
+		})
+		data = "2,5,e,,,"
+		t.Run(data, func(t *testing.T) {
+			if _, err := conn.Exec(query+nullif, srv.URL); !testutils.IsError(err, `row 1: generate insert row: null value in column "d" violates not-null constraint`) {
+				t.Fatalf("unexpected: %v", err)
+			}
+		})
 		data = "2,,e,,,"
-		if _, err := conn.Exec(query+nullif, srv.URL); !testutils.IsError(err, `"b" violates not-null constraint`) {
-			t.Fatalf("unexpected: %v", err)
-		}
+		t.Run(data, func(t *testing.T) {
+			if _, err := conn.Exec(query+nullif, srv.URL); !testutils.IsError(err, `"b" violates not-null constraint`) {
+				t.Fatalf("unexpected: %v", err)
+			}
+		})
 
 		data = "2,5,,,,"
-		if _, err := conn.Exec(query+nullif, srv.URL); !testutils.IsError(err, `"c" violates not-null constraint`) {
-			t.Fatalf("unexpected: %v", err)
-		}
+		t.Run(data, func(t *testing.T) {
+			if _, err := conn.Exec(query+nullif, srv.URL); !testutils.IsError(err, `"c" violates not-null constraint`) {
+				t.Fatalf("unexpected: %v", err)
+			}
+		})
 
-		data = "2,5,e,,,"
-		sqlDB.Exec(t, query+nullif, srv.URL)
-		sqlDB.CheckQueryResults(t,
-			`SELECT * FROM t`,
-			sqlDB.QueryStr(t, `SELECT 2, 5, 'e', NULL, NULL, NULL`),
-		)
+		data = "2,5,e,-1,,"
+		t.Run(data, func(t *testing.T) {
+			sqlDB.Exec(t, query+nullif, srv.URL)
+			sqlDB.CheckQueryResults(t,
+				`SELECT * FROM t`,
+				sqlDB.QueryStr(t, `SELECT 2, 5, 'e', -1, NULL, NULL`),
+			)
+		})
 	})
 }
 

pkg/ccl/importccl/load.go

@@ -161,6 +161,8 @@ func Load(
 			// only uses txn for resolving FKs and interleaved tables, neither of which
 			// are present here. Ditto for the schema accessor.
 			var txn *client.Txn
+			// At this point the CREATE statements in the loaded SQL do not
+			// use the SERIAL type so we need not process SERIAL types here.
 			desc, err := sql.MakeTableDesc(ctx, txn, nil /* vt */, st, s, dbDesc.ID,
 				0 /* table ID */, ts, privs, affected, nil, &evalCtx)
 			if err != nil {

pkg/ccl/importccl/read_import_mysql.go

@@ -304,6 +304,12 @@ func mysqlTableToCockroach(
 		if err != nil {
 			return nil, nil, err
 		}
+		// The new types in the table imported from MySQL do not (yet?)
+		// use SERIAL so we need not process SERIAL types here.
+		//
+		// If/when we extend this functionality to support MySQL'sAUTO
+		// INCREMENT, this will need to be extended -- see the comments on
+		// MakeColumnDefDescs().
 		col, _, _, err := sqlbase.MakeColumnDefDescs(def, &tree.SemaContext{}, evalCtx)
 		if err != nil {
 			return nil, nil, err

pkg/sql/alter_table.go

@@ -115,6 +115,18 @@ func (n *alterTableNode) startExec(params runParams) error {
 				return pgerror.Unimplemented(
 					"alter add fk", "adding a REFERENCES constraint via ALTER not supported")
 			}
+
+			newDef, seqDbDesc, seqName, seqOpts, err := params.p.processSerialInColumnDef(params.ctx, d, tn)
+			if err != nil {
+				return err
+			}
+			if seqName != nil {
+				if err := doCreateSequence(params, n.n.String(), seqDbDesc, seqName, seqOpts); err != nil {
+					return err
+				}
+			}
+			d = newDef
+
 			col, idx, expr, err := sqlbase.MakeColumnDefDescs(d, &params.p.semaCtx, params.EvalContext())
 			if err != nil {
 				return err

pkg/sql/coltypes/aliases.go

@@ -51,10 +51,6 @@ var (
 	Serial4 = &TInt{Name: "SERIAL4"}
 	// Serial8 is an immutable T instance.
 	Serial8 = &TInt{Name: "SERIAL8"}
-	// SmallSerial is an immutable T instance.
-	SmallSerial = &TInt{Name: "SMALLSERIAL"}
-	// BigSerial is an immutable T instance.
-	BigSerial = &TInt{Name: "BIGSERIAL"}
 
 	// Real is an immutable T instance.
 	Real = &TFloat{Name: "REAL", Width: 32}

pkg/sql/coltypes/arith.go

@@ -53,12 +53,10 @@ func (node *TInt) Format(buf *bytes.Buffer, f lex.EncodeFlags) {
 }
 
 var serialIntTypes = map[string]struct{}{
-	SmallSerial.Name: {},
-	Serial.Name:      {},
-	BigSerial.Name:   {},
-	Serial2.Name:     {},
-	Serial4.Name:     {},
-	Serial8.Name:     {},
+	Serial.Name:  {},
+	Serial2.Name: {},
+	Serial4.Name: {},
+	Serial8.Name: {},
 }
 
 // IsSerial returns true when this column should be given a DEFAULT of a unique,

pkg/sql/conn_executor.go

@@ -441,14 +441,15 @@ func (sp sessionParams) sessionData(
 		curDb = sessiondata.DefaultDatabaseName
 	}
 	sd := sessiondata.SessionData{
-		ApplicationName: sp.args.ApplicationName,
-		Database:        curDb,
-		DistSQLMode:     sessiondata.DistSQLExecMode(DistSQLClusterExecMode.Get(&settings.SV)),
-		OptimizerMode:   sessiondata.OptimizerMode(OptimizerClusterMode.Get(&settings.SV)),
-		SearchPath:      sqlbase.DefaultSearchPath,
-		User:            sp.args.User,
-		RemoteAddr:      sp.args.RemoteAddr,
-		SequenceState:   sessiondata.NewSequenceState(),
+		ApplicationName:         sp.args.ApplicationName,
+		Database:                curDb,
+		DistSQLMode:             sessiondata.DistSQLExecMode(DistSQLClusterExecMode.Get(&settings.SV)),
+		OptimizerMode:           sessiondata.OptimizerMode(OptimizerClusterMode.Get(&settings.SV)),
+		SerialNormalizationMode: sessiondata.SerialNormalizationMode(SerialNormalizationMode.Get(&settings.SV)),
+		SearchPath:              sqlbase.DefaultSearchPath,
+		User:                    sp.args.User,
+		RemoteAddr:              sp.args.RemoteAddr,
+		SequenceState:           sessiondata.NewSequenceState(),
 		DataConversion: sessiondata.DataConversionConfig{
 			Location: time.UTC,
 		},

pkg/sql/create_sequence.go

@@ -56,10 +56,8 @@ func (p *planner) CreateSequence(ctx context.Context, n *tree.CreateSequence) (p
 }
 
 func (n *createSequenceNode) startExec(params runParams) error {
-	seqName := n.n.Name.TableName().Table()
-	tKey := tableKey{parentID: n.dbDesc.ID, name: seqName}
-	key := tKey.Key()
-	if exists, err := descExists(params.ctx, params.p.txn, key); err == nil && exists {
+	tKey := getSequenceKey(n.dbDesc, n.n.Name.TableName().Table())
+	if exists, err := descExists(params.ctx, params.p.txn, tKey.Key()); err == nil && exists {
 		if n.n.IfNotExists {
 			// If the sequence exists but the user specified IF NOT EXISTS, return without doing anything.
 			return nil
@@ -69,23 +67,40 @@ func (n *createSequenceNode) startExec(params runParams) error {
 		return err
 	}
 
+	return doCreateSequence(params, n.n.String(), n.dbDesc, n.n.Name.TableName(), n.n.Options)
+}
+
+func getSequenceKey(dbDesc *DatabaseDescriptor, seqName string) tableKey {
+	return tableKey{parentID: dbDesc.ID, name: seqName}
+}
+
+// doCreateSequence performs the creation of a sequence in KV. The
+// context argument is a string to use in the event log.
+func doCreateSequence(
+	params runParams,
+	context string,
+	dbDesc *DatabaseDescriptor,
+	name *ObjectName,
+	opts tree.SequenceOptions,
+) error {
 	id, err := GenerateUniqueDescID(params.ctx, params.p.ExecCfg().DB)
 	if err != nil {
 		return err
 	}
 
 	// Inherit permissions from the database descriptor.
-	privs := n.dbDesc.GetPrivileges()
+	privs := dbDesc.GetPrivileges()
 
-	desc, err := n.makeSequenceTableDesc(params, seqName, n.dbDesc.ID, id, privs)
+	desc, err := MakeSequenceTableDesc(name.Table(), opts,
+		dbDesc.ID, id, params.p.txn.CommitTimestamp(), privs, params.EvalContext().Settings)
 	if err != nil {
 		return err
 	}
 
-	if err = desc.ValidateTable(params.EvalContext().Settings); err != nil {
-		return err
-	}
+	// makeSequenceTableDesc already validates the table. No call to
+	// desc.ValidateTable() needed here.
 
+	key := getSequenceKey(dbDesc, name.Table()).Key()
 	if err = params.p.createDescriptorWithID(params.ctx, key, id, &desc); err != nil {
 		return err
 	}
@@ -117,7 +132,7 @@ func (n *createSequenceNode) startExec(params runParams) error {
 			SequenceName string
 			Statement    string
 			User         string
-		}{n.n.Name.TableName().FQString(), n.n.String(), params.SessionData().User},
+		}{name.FQString(), context, params.SessionData().User},
 	)
 }
 
@@ -130,16 +145,6 @@ const (
 	sequenceColumnName = "value"
 )
 
-func (n *createSequenceNode) makeSequenceTableDesc(
-	params runParams,
-	sequenceName string,
-	parentID sqlbase.ID,
-	id sqlbase.ID,
-	privileges *sqlbase.PrivilegeDescriptor,
-) (sqlbase.TableDescriptor, error) {
-	return MakeSequenceTableDesc(sequenceName, n.n.Options, parentID, id, params.p.txn.CommitTimestamp(), privileges, params.EvalContext().Settings)
-}
-
 // MakeSequenceTableDesc creates a sequence descriptor.
 func MakeSequenceTableDesc(
 	sequenceName string,