diff --git a/pkg/ccl/importccl/read_import_mysql.go b/pkg/ccl/importccl/read_import_mysql.go index 62950a297cd4..3fab0b17b957 100644 --- a/pkg/ccl/importccl/read_import_mysql.go +++ b/pkg/ccl/importccl/read_import_mysql.go @@ -430,6 +430,7 @@ func mysqlTableToCockroach( privilegeDesc := descpb.NewBasePrivilegeDescriptor(owner) seqDesc, err = sql.NewSequenceTableDesc( ctx, + nil, /* planner */ seqName, opts, parentDB.GetID(), @@ -438,7 +439,6 @@ func mysqlTableToCockroach( time, privilegeDesc, tree.PersistencePermanent, - nil, /* params */ // If this is multi-region, this will get added by WriteDescriptors. false, /* isMultiRegion */ ) diff --git a/pkg/ccl/importccl/read_import_pgdump.go b/pkg/ccl/importccl/read_import_pgdump.go index ed2cf3192066..53a7743ad18f 100644 --- a/pkg/ccl/importccl/read_import_pgdump.go +++ b/pkg/ccl/importccl/read_import_pgdump.go @@ -321,6 +321,7 @@ func createPostgresSequences( } desc, err := sql.NewSequenceTableDesc( ctx, + nil, /* planner */ schemaAndTableName.table, seq.Options, parentID, @@ -329,7 +330,6 @@ func createPostgresSequences( hlc.Timestamp{WallTime: walltime}, descpb.NewBasePrivilegeDescriptor(owner), tree.PersistencePermanent, - nil, /* params */ // If this is multi-region, this will get added by WriteDescriptors. false, /* isMultiRegion */ ) diff --git a/pkg/ccl/importccl/testutils_test.go b/pkg/ccl/importccl/testutils_test.go index b6c9572b5914..4437e053db42 100644 --- a/pkg/ccl/importccl/testutils_test.go +++ b/pkg/ccl/importccl/testutils_test.go @@ -60,6 +60,7 @@ func descForTable( priv := descpb.NewBasePrivilegeDescriptor(security.AdminRoleName()) desc, err := sql.NewSequenceTableDesc( ctx, + nil, /* planner */ name, tree.SequenceOptions{}, parent, @@ -68,7 +69,6 @@ func descForTable( ts, priv, tree.PersistencePermanent, - nil, /* params */ false, /* isMultiRegion */ ) if err != nil { diff --git a/pkg/sql/add_column.go b/pkg/sql/add_column.go index 43f9f7ec2ae2..67629328066e 100644 --- a/pkg/sql/add_column.go +++ b/pkg/sql/add_column.go @@ -53,20 +53,24 @@ func (p *planner) addColumnImpl( return err } + var colOwnedSeqDesc *tabledesc.Mutable newDef, seqPrefix, seqName, seqOpts, err := params.p.processSerialLikeInColumnDef(params.ctx, d, tn) if err != nil { return err } if seqName != nil { - if err := doCreateSequence( - params, + colOwnedSeqDesc, err = doCreateSequence( + params.ctx, + params.p, + params.SessionData(), seqPrefix.Database, seqPrefix.Schema, seqName, n.tableDesc.Persistence(), seqOpts, tree.AsStringWithFQNames(n.n, params.Ann()), - ); err != nil { + ) + if err != nil { return err } } @@ -103,27 +107,6 @@ func (p *planner) addColumnImpl( } } - // If the new column has a DEFAULT or an ON UPDATE expression that uses a - // sequence, add references between its descriptor and this column descriptor. - if err := cdd.ForEachTypedExpr(func(expr tree.TypedExpr) error { - changedSeqDescs, err := maybeAddSequenceDependencies( - params.ctx, params.ExecCfg().Settings, params.p, n.tableDesc, col, expr, nil, - ) - if err != nil { - return err - } - for _, changedSeqDesc := range changedSeqDescs { - if err := params.p.writeSchemaChange( - params.ctx, changedSeqDesc, descpb.InvalidMutationID, tree.AsStringWithFQNames(n.n, params.Ann()), - ); err != nil { - return err - } - } - return nil - }); err != nil { - return err - } - // We're checking to see if a user is trying add a non-nullable column without a default to a // non empty table by scanning the primary index span with a limit of 1 to see if any key exists. if !col.Nullable && (col.DefaultExpr == nil && !col.IsComputed()) { @@ -176,16 +159,45 @@ func (p *planner) addColumnImpl( n.tableDesc.SetPrimaryIndex(primaryIndex) } - // Zone configuration logic is only required for REGIONAL BY ROW tables - // with newly created indexes. - if n.tableDesc.IsLocalityRegionalByRow() && idx != nil { - // We need to allocate new IDs for the created columns and indexes - // in case we need to configure their zone partitioning. - // This must be done after every object is created. - if err := n.tableDesc.AllocateIDs(params.ctx); err != nil { + // We need to allocate new ID for the created column in order to correctly + // assign sequence ownership. + if err := n.tableDesc.AllocateIDs(params.ctx); err != nil { + return err + } + + // If the new column has a DEFAULT or an ON UPDATE expression that uses a + // sequence, add references between its descriptor and this column descriptor. + if err := cdd.ForEachTypedExpr(func(expr tree.TypedExpr) error { + changedSeqDescs, err := maybeAddSequenceDependencies( + params.ctx, params.ExecCfg().Settings, params.p, n.tableDesc, col, expr, nil, + ) + if err != nil { return err } + for _, changedSeqDesc := range changedSeqDescs { + // `colOwnedSeqDesc` and `changedSeqDesc` should refer to a same instance. + // But we still want to use the right copy to write a schema change for by + // using `changedSeqDesc` just in case the assumption became false in the + // future. + if colOwnedSeqDesc != nil && colOwnedSeqDesc.ID == changedSeqDesc.ID { + if err := setSequenceOwner(changedSeqDesc, d.Name, desc); err != nil { + return err + } + } + if err := params.p.writeSchemaChange( + params.ctx, changedSeqDesc, descpb.InvalidMutationID, tree.AsStringWithFQNames(n.n, params.Ann()), + ); err != nil { + return err + } + } + return nil + }); err != nil { + return err + } + // Zone configuration logic is only required for REGIONAL BY ROW tables + // with newly created indexes. + if n.tableDesc.IsLocalityRegionalByRow() && idx != nil { // Configure zone configuration if required. This must happen after // all the IDs have been allocated. if err := p.configureZoneConfigForNewIndexPartitioning( diff --git a/pkg/sql/alter_sequence.go b/pkg/sql/alter_sequence.go index 3f30d2bc58f1..13a54c57f12c 100644 --- a/pkg/sql/alter_sequence.go +++ b/pkg/sql/alter_sequence.go @@ -82,10 +82,11 @@ func (n *alterSequenceNode) startExec(params runParams) error { } } if err := assignSequenceOptions( + params.ctx, + params.p, desc.SequenceOpts, n.n.Options, false, /* setDefaults */ - ¶ms, desc.GetID(), desc.ParentID, existingType, diff --git a/pkg/sql/create_sequence.go b/pkg/sql/create_sequence.go index 91c518894179..82fca241bc39 100644 --- a/pkg/sql/create_sequence.go +++ b/pkg/sql/create_sequence.go @@ -12,6 +12,7 @@ package sql import ( "context" + "fmt" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv" @@ -25,6 +26,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" "github.com/cockroachdb/cockroach/pkg/sql/privilege" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" "github.com/cockroachdb/cockroach/pkg/sql/types" @@ -80,33 +82,37 @@ func (n *createSequenceNode) startExec(params runParams) error { return err } - return doCreateSequence( - params, n.dbDesc, schemaDesc, &n.n.Name, n.n.Persistence, n.n.Options, + _, err = doCreateSequence( + params.ctx, params.p, params.SessionData(), n.dbDesc, schemaDesc, &n.n.Name, n.n.Persistence, n.n.Options, tree.AsStringWithFQNames(n.n, params.Ann()), ) + + return err } // 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, + ctx context.Context, + p *planner, + sessionData *sessiondata.SessionData, dbDesc catalog.DatabaseDescriptor, scDesc catalog.SchemaDescriptor, name *tree.TableName, persistence tree.Persistence, opts tree.SequenceOptions, jobDesc string, -) error { - id, err := descidgen.GenerateUniqueDescID(params.ctx, params.p.ExecCfg().DB, params.p.ExecCfg().Codec) +) (*tabledesc.Mutable, error) { + id, err := descidgen.GenerateUniqueDescID(ctx, p.ExecCfg().DB, p.ExecCfg().Codec) if err != nil { - return err + return nil, err } privs := catprivilege.CreatePrivilegesFromDefaultPrivileges( dbDesc.GetDefaultPrivilegeDescriptor(), scDesc.GetDefaultPrivilegeDescriptor(), dbDesc.GetID(), - params.SessionData().User(), + sessionData.User(), tree.Sequences, dbDesc.GetPrivileges(), ) @@ -122,7 +128,8 @@ func doCreateSequence( // currently relied on in import and restore code and tests. var creationTime hlc.Timestamp desc, err := NewSequenceTableDesc( - params.ctx, + ctx, + p, name.Object(), opts, dbDesc.GetID(), @@ -131,42 +138,99 @@ func doCreateSequence( creationTime, privs, persistence, - ¶ms, dbDesc.IsMultiRegion(), ) if err != nil { - return err + return nil, err } // makeSequenceTableDesc already validates the table. No call to // desc.ValidateSelf() needed here. - key := catalogkeys.MakeObjectNameKey(params.ExecCfg().Codec, dbDesc.GetID(), scDesc.GetID(), name.Object()) - if err = params.p.createDescriptorWithID(params.ctx, key, id, desc, jobDesc); err != nil { - return err + key := catalogkeys.MakeObjectNameKey(p.ExecCfg().Codec, dbDesc.GetID(), scDesc.GetID(), name.Object()) + if err = p.createDescriptorWithID(ctx, key, id, desc, jobDesc); err != nil { + return nil, err } // Initialize the sequence value. - seqValueKey := params.ExecCfg().Codec.SequenceKey(uint32(id)) + seqValueKey := p.ExecCfg().Codec.SequenceKey(uint32(id)) b := &kv.Batch{} b.Inc(seqValueKey, desc.SequenceOpts.Start-desc.SequenceOpts.Increment) - if err := params.p.txn.Run(params.ctx, b); err != nil { - return err + if err := p.txn.Run(ctx, b); err != nil { + return nil, err } - if err := validateDescriptor(params.ctx, params.p, desc); err != nil { - return err + if err := validateDescriptor(ctx, p, desc); err != nil { + return nil, err } // Log Create Sequence event. This is an auditable log event and is // recorded in the same transaction as the table descriptor update. - return params.p.logEvent(params.ctx, + return desc, p.logEvent(ctx, desc.ID, &eventpb.CreateSequence{ SequenceName: name.FQString(), }) } +func createSequencesForSerialColumns( + ctx context.Context, + p *planner, + sessionData *sessiondata.SessionData, + db catalog.DatabaseDescriptor, + sc catalog.SchemaDescriptor, + n *tree.CreateTable, +) (map[tree.Name]*tabledesc.Mutable, error) { + colNameToSeqDesc := make(map[tree.Name]*tabledesc.Mutable) + createStmt := n + ensureCopy := func() { + if createStmt == n { + newCreateStmt := *n + n.Defs = append(tree.TableDefs(nil), n.Defs...) + createStmt = &newCreateStmt + } + } + + tn := tree.MakeTableNameFromPrefix(catalog.ResolvedObjectPrefix{ + Database: db, + Schema: sc, + }.NamePrefix(), tree.Name(n.Table.Table())) + for i, def := range n.Defs { + d, ok := def.(*tree.ColumnTableDef) + if !ok { + continue + } + newDef, prefix, seqName, seqOpts, err := p.processSerialLikeInColumnDef(ctx, d, &tn) + if err != nil { + return nil, err + } + // TODO (lucy): Have more consistent/informative names for dependent jobs. + if seqName != nil { + seqDesc, err := doCreateSequence( + ctx, + p, + sessionData, + prefix.Database, + prefix.Schema, + seqName, + n.Persistence, + seqOpts, + fmt.Sprintf("creating sequence %s for new table %s", seqName, n.Table.Table()), + ) + if err != nil { + return nil, err + } + colNameToSeqDesc[d.Name] = seqDesc + } + if d != newDef { + ensureCopy() + n.Defs[i] = newDef + } + } + + return colNameToSeqDesc, nil +} + func (*createSequenceNode) Next(runParams) (bool, error) { return false, nil } func (*createSequenceNode) Values() tree.Datums { return tree.Datums{} } func (*createSequenceNode) Close(context.Context) {} @@ -174,6 +238,7 @@ func (*createSequenceNode) Close(context.Context) {} // NewSequenceTableDesc creates a sequence descriptor. func NewSequenceTableDesc( ctx context.Context, + p *planner, sequenceName string, sequenceOptions tree.SequenceOptions, parentID descpb.ID, @@ -182,7 +247,6 @@ func NewSequenceTableDesc( creationTime hlc.Timestamp, privileges *descpb.PrivilegeDescriptor, persistence tree.Persistence, - params *runParams, isMultiRegion bool, ) (*tabledesc.Mutable, error) { desc := tabledesc.InitTableDescriptor( @@ -227,10 +291,11 @@ func NewSequenceTableDesc( Increment: 1, } if err := assignSequenceOptions( + ctx, + p, opts, sequenceOptions, true, /* setDefaults */ - params, id, parentID, nil, /* existingType */ diff --git a/pkg/sql/create_table.go b/pkg/sql/create_table.go index f3edff38f7df..3f2d1d36bebb 100644 --- a/pkg/sql/create_table.go +++ b/pkg/sql/create_table.go @@ -2185,16 +2185,6 @@ func newTableDesc( privileges *descpb.PrivilegeDescriptor, affected map[descpb.ID]*tabledesc.Mutable, ) (ret *tabledesc.Mutable, err error) { - // Process any SERIAL columns to remove the SERIAL type, - // as required by NewTableDesc. - createStmt := n - ensureCopy := func() { - if createStmt == n { - newCreateStmt := *n - n.Defs = append(tree.TableDefs(nil), n.Defs...) - createStmt = &newCreateStmt - } - } newDefs, err := replaceLikeTableOpts(n, params) if err != nil { return nil, err @@ -2207,37 +2197,13 @@ func newTableDesc( n.Defs = newDefs } - tn := tree.MakeTableNameFromPrefix(catalog.ResolvedObjectPrefix{ - Database: db, - Schema: sc, - }.NamePrefix(), tree.Name(n.Table.Table())) - for i, def := range n.Defs { - d, ok := def.(*tree.ColumnTableDef) - if !ok { - continue - } - newDef, prefix, seqName, seqOpts, err := params.p.processSerialLikeInColumnDef(params.ctx, d, &tn) - if err != nil { - return nil, err - } - // TODO (lucy): Have more consistent/informative names for dependent jobs. - if seqName != nil { - if err := doCreateSequence( - params, - prefix.Database, - prefix.Schema, - seqName, - n.Persistence, - seqOpts, - fmt.Sprintf("creating sequence %s for new table %s", seqName, n.Table.Table()), - ); err != nil { - return nil, err - } - } - if d != newDef { - ensureCopy() - n.Defs[i] = newDef - } + // Process any SERIAL columns to remove the SERIAL type, as required by + // NewTableDesc. + colNameToOwnedSeq, err := createSequencesForSerialColumns( + params.ctx, params.p, params.SessionData(), db, sc, n, + ) + if err != nil { + return nil, err } var regionConfig *multiregion.RegionConfig @@ -2273,6 +2239,17 @@ func newTableDesc( ) }) + // We need to ensure sequence ownerships so that column owned sequences are + // correctly dropped when a column/table is dropped. + for colName, seqDesc := range colNameToOwnedSeq { + // When a table is first created, `affected` includes all the newly created + // sequenced. So `affectedSeqDesc` should be always non-nil. + affectedSeqDesc := affected[seqDesc.ID] + if err := setSequenceOwner(affectedSeqDesc, colName, ret); err != nil { + return nil, err + } + } + return ret, err } @@ -2620,3 +2597,32 @@ func checkTypeIsSupported(ctx context.Context, settings *cluster.Settings, typ * } return nil } + +// setSequenceOwner adds sequence id to the sequence id list owned by a column +// and set ownership values of sequence options. +func setSequenceOwner( + seqDesc *tabledesc.Mutable, colName tree.Name, table *tabledesc.Mutable, +) error { + if !seqDesc.IsSequence() { + return errors.Errorf("%s is not a sequence", seqDesc.Name) + } + + col, err := table.FindColumnWithName(colName) + if err != nil { + return err + } + found := false + for _, seqID := range col.ColumnDesc().OwnsSequenceIds { + if seqID == seqDesc.ID { + found = true + break + } + } + if !found { + col.ColumnDesc().OwnsSequenceIds = append(col.ColumnDesc().OwnsSequenceIds, seqDesc.ID) + } + seqDesc.SequenceOpts.SequenceOwner.OwnerTableID = table.ID + seqDesc.SequenceOpts.SequenceOwner.OwnerColumnID = col.GetID() + + return nil +} diff --git a/pkg/sql/logictest/testdata/logic_test/alter_table b/pkg/sql/logictest/testdata/logic_test/alter_table index 102065d17748..647ade8fa852 100644 --- a/pkg/sql/logictest/testdata/logic_test/alter_table +++ b/pkg/sql/logictest/testdata/logic_test/alter_table @@ -2226,3 +2226,88 @@ alter table const_tbl drop constraint "steve"; alter table const_tbl add constraint "steve" CHECK (a > 100); alter table const_tbl add constraint "steve" CHECK (a > 100); COMMIT; + +# This is needed for following tests not to run in the aborted transaction +# above. +statement ok +rollback + +# Test sequence is drop when a owner table/column is dropped +subtest test_serial_ownership_add_column + +statement ok +SET serial_normalization = sql_sequence; + +statement ok +CREATE TABLE test_serial ( + a INT PRIMARY KEY +); + +statement ok +ALTER TABLE test_serial ADD COLUMN b SERIAL; + +query ITTT colnames +SELECT l.table_id, l.name, l.state, r.refobjid +FROM ( + SELECT table_id, name, state + FROM crdb_internal.tables WHERE name + LIKE 'test_serial%' AND state = 'PUBLIC' +) l +LEFT JOIN pg_catalog.pg_depend r ON l.table_id = r.objid; +---- +table_id name state refobjid +155 test_serial_b_seq PUBLIC 154 +154 test_serial PUBLIC NULL + +statement ok +DROP TABLE test_serial; + +query ITTT colnames +SELECT l.table_id, l.name, l.state, r.refobjid +FROM ( + SELECT table_id, name, state + FROM crdb_internal.tables WHERE name + LIKE 'test_serial%' AND state = 'PUBLIC' +) l +LEFT JOIN pg_catalog.pg_depend r ON l.table_id = r.objid; +---- +table_id name state refobjid + +statement ok +CREATE TABLE test_serial ( + a INT PRIMARY KEY +); + +statement ok +ALTER TABLE test_serial ADD COLUMN b SERIAL; + +query ITTT colnames +SELECT l.table_id, l.name, l.state, r.refobjid +FROM ( + SELECT table_id, name, state + FROM crdb_internal.tables WHERE name + LIKE 'test_serial%' AND state = 'PUBLIC' +) l +LEFT JOIN pg_catalog.pg_depend r ON l.table_id = r.objid; +---- +table_id name state refobjid +157 test_serial_b_seq PUBLIC 156 +156 test_serial PUBLIC NULL + +statement ok +ALTER TABLE test_serial DROP COLUMN b; + +query ITTT colnames +SELECT l.table_id, l.name, l.state, r.refobjid +FROM ( + SELECT table_id, name, state + FROM crdb_internal.tables WHERE name + LIKE 'test_serial%' AND state = 'PUBLIC' +) l +LEFT JOIN pg_catalog.pg_depend r ON l.table_id = r.objid; +---- +table_id name state refobjid +156 test_serial PUBLIC NULL + +statement ok +DROP TABLE test_serial; diff --git a/pkg/sql/logictest/testdata/logic_test/create_table b/pkg/sql/logictest/testdata/logic_test/create_table index 6d9201d2964f..12484244e374 100644 --- a/pkg/sql/logictest/testdata/logic_test/create_table +++ b/pkg/sql/logictest/testdata/logic_test/create_table @@ -636,3 +636,79 @@ CREATE TABLE serial_test4 (id SERIAL4, temp string) statement ok CREATE TABLE regression_73648 AS select * from [SHOW CLUSTER QUERIES] + +# Test sequence is drop when a owner table/column is dropped +subtest test_serial_ownership_create_table + +statement ok +SET serial_normalization = sql_sequence; + +statement ok +CREATE TABLE test_serial ( + a INT PRIMARY KEY, + b SERIAL +); + +query ITTT colnames +SELECT l.table_id, l.name, l.state, r.refobjid +FROM ( + SELECT table_id, name, state + FROM crdb_internal.tables WHERE name + LIKE 'test_serial%' AND state = 'PUBLIC' +) l +LEFT JOIN pg_catalog.pg_depend r ON l.table_id = r.objid; +---- +table_id name state refobjid +124 test_serial_b_seq PUBLIC 123 +123 test_serial PUBLIC NULL + +statement ok +DROP TABLE test_serial; + +query ITTT colnames +SELECT l.table_id, l.name, l.state, r.refobjid +FROM ( + SELECT table_id, name, state + FROM crdb_internal.tables WHERE name + LIKE 'test_serial%' AND state = 'PUBLIC' +) l +LEFT JOIN pg_catalog.pg_depend r ON l.table_id = r.objid; +---- +table_id name state refobjid + +statement ok +CREATE TABLE test_serial ( + a INT PRIMARY KEY, + b SERIAL +); + +query ITTT colnames +SELECT l.table_id, l.name, l.state, r.refobjid +FROM ( + SELECT table_id, name, state + FROM crdb_internal.tables WHERE name + LIKE 'test_serial%' AND state = 'PUBLIC' +) l +LEFT JOIN pg_catalog.pg_depend r ON l.table_id = r.objid; +---- +table_id name state refobjid +126 test_serial_b_seq PUBLIC 125 +125 test_serial PUBLIC NULL + +statement ok +ALTER TABLE test_serial DROP COLUMN b; + +query ITTT colnames +SELECT l.table_id, l.name, l.state, r.refobjid +FROM ( + SELECT table_id, name, state + FROM crdb_internal.tables WHERE name + LIKE 'test_serial%' AND state = 'PUBLIC' +) l +LEFT JOIN pg_catalog.pg_depend r ON l.table_id = r.objid; +---- +table_id name state refobjid +125 test_serial PUBLIC NULL + +statement ok +DROP TABLE test_serial; diff --git a/pkg/sql/logictest/testdata/logic_test/pg_catalog b/pkg/sql/logictest/testdata/logic_test/pg_catalog index 6aecacf3b510..335796e57c25 100644 --- a/pkg/sql/logictest/testdata/logic_test/pg_catalog +++ b/pkg/sql/logictest/testdata/logic_test/pg_catalog @@ -1410,12 +1410,14 @@ FROM pg_catalog.pg_depend ORDER BY objid ---- classid objid objsubid refclassid refobjid refobjsubid deptype +4294967127 61 0 4294967130 60 14 a +4294967127 62 0 4294967130 60 15 a 4294967127 1257009153 0 4294967130 0 0 n 4294967127 3132697166 0 4294967130 0 0 n -4294967084 3300576943 0 4294967130 60 3 n -4294967084 3300576943 0 4294967130 60 4 n 4294967084 3300576943 0 4294967130 60 1 n 4294967084 3300576943 0 4294967130 60 2 n +4294967084 3300576943 0 4294967130 60 3 n +4294967084 3300576943 0 4294967130 60 4 n 4294967127 3823689858 0 4294967130 1229708770 0 n 4294967127 4221688865 0 4294967130 1229708771 0 n @@ -1448,6 +1450,8 @@ t1 r t1 r t1 r t1 r +t1 r +t1 r t1_a_key i diff --git a/pkg/sql/logictest/testdata/logic_test/serial b/pkg/sql/logictest/testdata/logic_test/serial index 905991e7f1de..e6fa85b19c79 100644 --- a/pkg/sql/logictest/testdata/logic_test/serial +++ b/pkg/sql/logictest/testdata/logic_test/serial @@ -222,23 +222,23 @@ query TT SHOW CREATE TABLE serial ---- serial CREATE TABLE public.serial ( - a INT8 NOT NULL DEFAULT nextval('public.serial_a_seq1'::REGCLASS), + a INT8 NOT NULL DEFAULT nextval('public.serial_a_seq'::REGCLASS), b INT8 NULL DEFAULT 7:::INT8, - c INT8 NOT NULL DEFAULT nextval('public.serial_c_seq3'::REGCLASS), + c INT8 NOT NULL DEFAULT nextval('public.serial_c_seq2'::REGCLASS), CONSTRAINT serial_pkey PRIMARY KEY (a ASC), UNIQUE INDEX serial_c_key (c ASC), FAMILY "primary" (a, b, c) ) query TT -SHOW CREATE SEQUENCE serial_a_seq1 +SHOW CREATE SEQUENCE serial_a_seq ---- -serial_a_seq1 CREATE SEQUENCE public.serial_a_seq1 MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1 +serial_a_seq CREATE SEQUENCE public.serial_a_seq MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1 query TT SELECT pg_get_serial_sequence('serial', 'a'), pg_get_serial_sequence('serial', 'b') ---- -public.serial_a_seq1 NULL +public.serial_a_seq NULL statement error relation "non_existent_tbl" does not exist SELECT pg_get_serial_sequence('non_existent_tbl', 'a') @@ -305,8 +305,8 @@ query TT SHOW CREATE TABLE smallbig ---- smallbig CREATE TABLE public.smallbig ( - a INT2 NOT NULL DEFAULT nextval('public.smallbig_a_seq1'::REGCLASS), - b INT8 NOT NULL DEFAULT nextval('public.smallbig_b_seq1'::REGCLASS), + a INT2 NOT NULL DEFAULT nextval('public.smallbig_a_seq'::REGCLASS), + b INT8 NOT NULL DEFAULT nextval('public.smallbig_b_seq'::REGCLASS), c INT8 NULL, rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), CONSTRAINT smallbig_pkey PRIMARY KEY (rowid ASC), @@ -328,9 +328,9 @@ query TT SHOW CREATE TABLE serials ---- serials CREATE TABLE public.serials ( - a INT2 NOT NULL DEFAULT nextval('public.serials_a_seq1'::REGCLASS), - b INT4 NOT NULL DEFAULT nextval('public.serials_b_seq1'::REGCLASS), - c INT8 NOT NULL DEFAULT nextval('public.serials_c_seq1'::REGCLASS), + a INT2 NOT NULL DEFAULT nextval('public.serials_a_seq'::REGCLASS), + b INT4 NOT NULL DEFAULT nextval('public.serials_b_seq'::REGCLASS), + c INT8 NOT NULL DEFAULT nextval('public.serials_c_seq'::REGCLASS), d INT8 NULL, rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), CONSTRAINT serials_pkey PRIMARY KEY (rowid ASC), diff --git a/pkg/sql/logictest/testdata/logic_test/temp_table b/pkg/sql/logictest/testdata/logic_test/temp_table index 77aa78c7b6bf..fcbe90f14baa 100644 --- a/pkg/sql/logictest/testdata/logic_test/temp_table +++ b/pkg/sql/logictest/testdata/logic_test/temp_table @@ -207,7 +207,13 @@ statement ok DROP TABLE perm_table; DROP TABLE ref_temp_table statement ok -DROP SEQUENCE pg_temp.temp_seq; DROP SEQUENCE pg_temp.ref_temp_table_a_seq; DROP TABLE a +DROP SEQUENCE pg_temp.temp_seq; DROP TABLE a + +query ITT +SELECT table_id, name, state +FROM crdb_internal.tables WHERE name +LIKE 'ref_temp_table%' AND state = 'PUBLIC' +---- statement ok SET serial_normalization='rowid' diff --git a/pkg/sql/sequence.go b/pkg/sql/sequence.go index 30770858a09c..ee52b51612fc 100644 --- a/pkg/sql/sequence.go +++ b/pkg/sql/sequence.go @@ -438,10 +438,11 @@ func setSequenceIntegerBounds( // assignSequenceOptions moves options from the AST node to the sequence options descriptor, // starting with defaults and overriding them with user-provided options. func assignSequenceOptions( + ctx context.Context, + p *planner, opts *descpb.TableDescriptor_SequenceOpts, optsNode tree.SequenceOptions, setDefaults bool, - params *runParams, sequenceID descpb.ID, sequenceParentID descpb.ID, existingType *types.T, @@ -557,25 +558,25 @@ func assignSequenceOptions( case tree.SeqOptVirtual: opts.Virtual = true case tree.SeqOptOwnedBy: - if params == nil { + if p == nil { return pgerror.Newf(pgcode.Internal, - "Trying to add/remove Sequence Owner without access to context") + "Trying to add/remove Sequence Owner outside of context of a planner") } // The owner is being removed if option.ColumnItemVal == nil { - if err := removeSequenceOwnerIfExists(params.ctx, params.p, sequenceID, opts); err != nil { + if err := removeSequenceOwnerIfExists(ctx, p, sequenceID, opts); err != nil { return err } } else { // The owner is being added/modified tableDesc, col, err := resolveColumnItemToDescriptors( - params.ctx, params.p, option.ColumnItemVal, + ctx, p, option.ColumnItemVal, ) if err != nil { return err } if tableDesc.ParentID != sequenceParentID && - !allowCrossDatabaseSeqOwner.Get(¶ms.p.execCfg.Settings.SV) { + !allowCrossDatabaseSeqOwner.Get(&p.execCfg.Settings.SV) { return errors.WithHintf( pgerror.Newf(pgcode.FeatureNotSupported, "OWNED BY cannot refer to other databases; (see the '%s' cluster setting)", @@ -587,10 +588,10 @@ func assignSequenceOptions( // want it to be. if opts.SequenceOwner.OwnerTableID != tableDesc.ID || opts.SequenceOwner.OwnerColumnID != col.GetID() { - if err := removeSequenceOwnerIfExists(params.ctx, params.p, sequenceID, opts); err != nil { + if err := removeSequenceOwnerIfExists(ctx, p, sequenceID, opts); err != nil { return err } - err := addSequenceOwner(params.ctx, params.p, option.ColumnItemVal, sequenceID, opts) + err := addSequenceOwner(ctx, p, option.ColumnItemVal, sequenceID, opts) if err != nil { return err } diff --git a/pkg/sql/temporary_schema.go b/pkg/sql/temporary_schema.go index c3814f80311c..f069b3063297 100644 --- a/pkg/sql/temporary_schema.go +++ b/pkg/sql/temporary_schema.go @@ -265,11 +265,16 @@ func cleanupSchemaObjects( databaseIDToTempSchemaID[uint32(desc.GetParentID())] = uint32(desc.GetParentSchemaID()) - if desc.GetSequenceOpts() != nil { + // If a sequence is owned by a table column, it is dropped when the owner + // table/column is dropped. So here we want to only drop sequences not + // owned. + if desc.IsSequence() && + desc.GetSequenceOpts().SequenceOwner.OwnerColumnID == 0 && + desc.GetSequenceOpts().SequenceOwner.OwnerTableID == 0 { sequences = append(sequences, desc.GetID()) } else if desc.GetViewQuery() != "" { views = append(views, desc.GetID()) - } else { + } else if !desc.IsSequence() { tables = append(tables, desc.GetID()) } } diff --git a/pkg/sql/testutils.go b/pkg/sql/testutils.go index 68b063cee571..d2d2fc606666 100644 --- a/pkg/sql/testutils.go +++ b/pkg/sql/testutils.go @@ -74,13 +74,13 @@ func CreateTestTableDescriptor( case *tree.CreateSequence: desc, err := NewSequenceTableDesc( ctx, + nil, /* planner */ n.Name.Table(), n.Options, parentID, keys.PublicSchemaID, id, hlc.Timestamp{}, /* creationTime */ privileges, tree.PersistencePermanent, - nil, /* params */ false, /* isMultiRegion */ ) return desc, err