diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index 0e13d722e434..24df1c61c833 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -523,12 +523,9 @@ func (n *alterTableNode) startExec(params runParams) error { return sqlerrors.NewUndefinedConstraintError(string(t.Constraint), n.tableDesc.Name) } switch c.GetConstraintValidity() { - case descpb.ConstraintValidity_Validated: + case descpb.ConstraintValidity_Validated, descpb.ConstraintValidity_Validating: // Nothing to do. continue - case descpb.ConstraintValidity_Validating: - return pgerror.Newf(pgcode.ObjectNotInPrerequisiteState, - "constraint %q in the middle of being added, try again later", t.Constraint) case descpb.ConstraintValidity_Dropping: return sqlerrors.NewUndefinedConstraintError(string(t.Constraint), n.tableDesc.Name) } diff --git a/pkg/sql/backfill.go b/pkg/sql/backfill.go index 8f1e4fa0de10..55eb7eaebcae 100644 --- a/pkg/sql/backfill.go +++ b/pkg/sql/backfill.go @@ -2667,10 +2667,9 @@ func getTargetTablesAndFk( if srcTable.Version > srcTable.ClusterVersion().Version { syntheticTable = srcTable } - for i := range srcTable.OutboundFKs { - def := &srcTable.OutboundFKs[i] - if def.Name == fkName { - fk = def + for _, outbounfFK := range srcTable.OutboundForeignKeys() { + if outbounfFK.GetName() == fkName { + fk = outbounfFK.ForeignKeyDesc() break } } @@ -2741,10 +2740,9 @@ func validateUniqueWithoutIndexConstraintInTxn( syntheticDescs = append(syntheticDescs, tableDesc) } var uc *descpb.UniqueWithoutIndexConstraint - for i := range tableDesc.UniqueWithoutIndexConstraints { - def := &tableDesc.UniqueWithoutIndexConstraints[i] - if def.Name == constraintName { - uc = def + for _, uwi := range tableDesc.UniqueConstraintsWithoutIndex() { + if uwi.GetName() == constraintName { + uc = uwi.UniqueWithoutIndexDesc() break } } diff --git a/pkg/sql/catalog/tabledesc/structured.go b/pkg/sql/catalog/tabledesc/structured.go index 875c20790404..4bc43feab1d5 100644 --- a/pkg/sql/catalog/tabledesc/structured.go +++ b/pkg/sql/catalog/tabledesc/structured.go @@ -1391,9 +1391,17 @@ func (desc *Mutable) MakeMutationComplete(m descpb.DescriptorMutation) error { break } } - case descpb.ConstraintValidity_Unvalidated: + case descpb.ConstraintValidity_Unvalidated, descpb.ConstraintValidity_Validated: // add the constraint to the list of check constraints on the table // descriptor. + // The constraint mutation's validity being VALIDATED is strange but could happen + // in the legacy schema changer when we do + // `ALTER TABLE ... ADD CONSTRAINT ... NOT VALID, VALIDATE CONSTRAINT ...` + // where `ADD CONSTRAINT ... NOT VALID` enqueues a mutation and + // `VALIDATE CONSTRAINT` change this mutation's validity to VALIDATED. + // We thus will run into this case later in the job for + // `ADD CONSTRAINT ... NOT VALID`, at which point we should just append it + // to the public slice and return. desc.Checks = append(desc.Checks, &t.Constraint.Check) default: return errors.AssertionFailedf("invalid constraint validity state: %d", t.Constraint.Check.Validity) @@ -1409,7 +1417,7 @@ func (desc *Mutable) MakeMutationComplete(m descpb.DescriptorMutation) error { break } } - case descpb.ConstraintValidity_Unvalidated: + case descpb.ConstraintValidity_Unvalidated, descpb.ConstraintValidity_Validated: // Takes care of adding the Foreign Key to the table index. Adding the // backreference to the referenced table index must be taken care of // in another call. @@ -1427,7 +1435,7 @@ func (desc *Mutable) MakeMutationComplete(m descpb.DescriptorMutation) error { break } } - case descpb.ConstraintValidity_Unvalidated: + case descpb.ConstraintValidity_Unvalidated, descpb.ConstraintValidity_Validated: // add the constraint to the list of unique without index constraints // on the table descriptor. desc.UniqueWithoutIndexConstraints = append( diff --git a/pkg/sql/logictest/testdata/logic_test/alter_table b/pkg/sql/logictest/testdata/logic_test/alter_table index 767e716e6a92..39aa0e77476e 100644 --- a/pkg/sql/logictest/testdata/logic_test/alter_table +++ b/pkg/sql/logictest/testdata/logic_test/alter_table @@ -2903,3 +2903,40 @@ t_96115 t_96115_pkey PRIMARY KEY true statement ok DROP TABLE t_96115; + +# This subtest tests the behavior of adding/dropping a constraint and validating +# the constraint in one ALTER TABLE statement. + +subtest 96648 + +statement ok +CREATE TABLE t_96648 (i INT PRIMARY KEY); + +statement ok +ALTER TABLE t_96648 ADD CONSTRAINT check_i CHECK (i > 0), VALIDATE CONSTRAINT check_i; + +statement ok +ALTER TABLE t_96648 ADD CONSTRAINT check_i1 CHECK (i > 0) NOT VALID, VALIDATE CONSTRAINT check_i1; + +query TTTTB colnames +SHOW CONSTRAINTS FROM t_96648 +---- +table_name constraint_name constraint_type details validated +t_96648 check_i CHECK CHECK ((i > 0)) true +t_96648 check_i1 CHECK CHECK ((i > 0)) true +t_96648 t_96648_pkey PRIMARY KEY PRIMARY KEY (i ASC) true + +statement ok +ALTER TABLE t_96648 ADD CONSTRAINT check_i2 CHECK (i > 0) NOT VALID; + +statement error pgcode 42704 constraint "check_i" of relation "t_96648" does not exist +ALTER TABLE t_96648 DROP CONSTRAINT check_i, VALIDATE CONSTRAINT check_i; + +statement error pgcode 42704 constraint "check_i2" of relation "t_96648" does not exist +ALTER TABLE t_96648 DROP CONSTRAINT check_i2, VALIDATE CONSTRAINT check_i2; + +statement ok +ALTER TABLE t_96648 DROP CONSTRAINT check_i, DROP CONSTRAINT check_i1, DROP CONSTRAINT check_i2; + +statement ok +DROP TABLE t_96648 diff --git a/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn b/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn index c2037b0e683a..03596e234868 100644 --- a/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn +++ b/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn @@ -831,14 +831,14 @@ CREATE TABLE orders2 ( INDEX (product) ) -# The constraint can't be validated with VALIDATE CONSTRAINT in the same transaction +# VALIDATE CONSTRAINT when a vanilla constraint is being added is a no-op. statement ok BEGIN statement ok ALTER TABLE orders2 ADD FOREIGN KEY (product) REFERENCES products -statement error constraint "orders2_product_fkey" in the middle of being added, try again later +statement ok ALTER TABLE orders2 VALIDATE CONSTRAINT orders2_product_fkey statement ok @@ -851,7 +851,7 @@ BEGIN statement ok ALTER TABLE orders2 ADD FOREIGN KEY (product) REFERENCES products -statement error constraint "orders2_product_fkey" in the middle of being added, try again later +statement error pq: referencing constraint "orders2_product_fkey_1" in the middle of being added, try again later ALTER TABLE orders2 DROP COLUMN product statement ok @@ -864,7 +864,7 @@ BEGIN statement ok ALTER TABLE orders2 ADD FOREIGN KEY (product) REFERENCES products -statement error constraint "orders2_product_fkey" in the middle of being added, try again later +statement error pq: referencing constraint "orders2_product_fkey_1" in the middle of being added, try again later DROP INDEX orders2@orders2_product_idx statement ok @@ -1051,7 +1051,7 @@ ALTER TABLE check_table ADD h INT statement ok ALTER TABLE check_table ADD CONSTRAINT h_0 CHECK (h > 0) -statement error constraint "h_0" in the middle of being added +statement ok ALTER TABLE check_table VALIDATE CONSTRAINT h_0 statement ok diff --git a/pkg/sql/schema_changer.go b/pkg/sql/schema_changer.go index 8d1dc65b9682..d3c9bd18f9da 100644 --- a/pkg/sql/schema_changer.go +++ b/pkg/sql/schema_changer.go @@ -1413,7 +1413,8 @@ func (sc *SchemaChanger) done(ctx context.Context) error { } } if fk := m.AsForeignKey(); fk != nil && fk.Adding() && - fk.GetConstraintValidity() == descpb.ConstraintValidity_Unvalidated { + (fk.GetConstraintValidity() == descpb.ConstraintValidity_Unvalidated || + fk.GetConstraintValidity() == descpb.ConstraintValidity_Validated) { // Add backreference on the referenced table (which could be the same table) backrefTable, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, fk.GetReferencedTableID()) if err != nil {