Skip to content

Commit

Permalink
Merge cockroachdb#37554
Browse files Browse the repository at this point in the history 
37554: sql: support ALTER COLUMN SET NOT NULL r=lucy-zhang a=lucy-zhang

This PR adds support for `ALTER TABLE ... ALTER COLUMN ... SET NOT NULL`, to
add a non-null constraint to an existing column. This is done by creating a
dummy CHECK constraint in the `Validating` state which moves through the states
of the schema changer, and setting `Nullable` to true on the column descriptor
only at the very end. Adding a constraint to an existing column requires the
constraint to be in a "write-only" state while validation occurs, and the
advantage of the approach in this PR is that we can reuse the existing CHECK
constraint states, where "write-only" corresponds to the state where the
constraint is on the table descriptor in the `Validating` state.

(As with FK and check constraints, we still need to process dropped constraints
in the schema changer. That's been left for a separate PR.)

Closes cockroachdb#28751.

Release note (sql change): Add support for `ALTER TABLE ... ALTER COLUMN ...
SET NOT NULL`, which adds a non-null constraint to an existing column.

Co-authored-by: Lucy Zhang <[email protected]>
  • Loading branch information
@lucy-zhang
craig[bot] and lucy-zhang committed Jun 19, 2019
2 parents b715f9f + 8fe4986 commit f6d75a5
Show file tree
Hide file tree
Showing 18 changed files with 590 additions and 242 deletions.
4 changes: 4 additions & 0 deletions docs/generated/sql/bnf/alter_column.bnf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ alter_onetable_stmt ::=
| 'ALTER' 'TABLE' table_name 'ALTER' column_name 'DROP' 'NOT' 'NULL'
| 'ALTER' 'TABLE' table_name 'ALTER' 'COLUMN' column_name 'DROP' 'STORED'
| 'ALTER' 'TABLE' table_name 'ALTER' column_name 'DROP' 'STORED'
| 'ALTER' 'TABLE' table_name 'ALTER' 'COLUMN' column_name 'SET' 'NOT' 'NULL'
| 'ALTER' 'TABLE' table_name 'ALTER' column_name 'SET' 'NOT' 'NULL'
| 'ALTER' 'TABLE' table_name 'ALTER' 'COLUMN' column_name 'SET' 'DATA' 'TYPE' typename 'COLLATE' collation_name 'USING' a_expr
| 'ALTER' 'TABLE' table_name 'ALTER' 'COLUMN' column_name 'SET' 'DATA' 'TYPE' typename 'COLLATE' collation_name
| 'ALTER' 'TABLE' table_name 'ALTER' 'COLUMN' column_name 'SET' 'DATA' 'TYPE' typename 'USING' a_expr
Expand All @@ -31,6 +33,8 @@ alter_onetable_stmt ::=
| 'ALTER' 'TABLE' 'IF' 'EXISTS' table_name 'ALTER' column_name 'DROP' 'NOT' 'NULL'
| 'ALTER' 'TABLE' 'IF' 'EXISTS' table_name 'ALTER' 'COLUMN' column_name 'DROP' 'STORED'
| 'ALTER' 'TABLE' 'IF' 'EXISTS' table_name 'ALTER' column_name 'DROP' 'STORED'
| 'ALTER' 'TABLE' 'IF' 'EXISTS' table_name 'ALTER' 'COLUMN' column_name 'SET' 'NOT' 'NULL'
| 'ALTER' 'TABLE' 'IF' 'EXISTS' table_name 'ALTER' column_name 'SET' 'NOT' 'NULL'
| 'ALTER' 'TABLE' 'IF' 'EXISTS' table_name 'ALTER' 'COLUMN' column_name 'SET' 'DATA' 'TYPE' typename 'COLLATE' collation_name 'USING' a_expr
| 'ALTER' 'TABLE' 'IF' 'EXISTS' table_name 'ALTER' 'COLUMN' column_name 'SET' 'DATA' 'TYPE' typename 'COLLATE' collation_name
| 'ALTER' 'TABLE' 'IF' 'EXISTS' table_name 'ALTER' 'COLUMN' column_name 'SET' 'DATA' 'TYPE' typename 'USING' a_expr
Expand Down
4 changes: 2 additions & 2 deletions docs/generated/sql/bnf/alter_table.bnf
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
alter_onetable_stmt ::=
'ALTER' 'TABLE' table_name ( ( ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by ) ) ( ( ',' ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by ) ) )* )
| 'ALTER' 'TABLE' 'IF' 'EXISTS' table_name ( ( ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by ) ) ( ( ',' ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by ) ) )* )
'ALTER' 'TABLE' table_name ( ( ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by ) ) ( ( ',' ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by ) ) )* )
| 'ALTER' 'TABLE' 'IF' 'EXISTS' table_name ( ( ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by ) ) ( ( ',' ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by ) ) )* )
1 change: 1 addition & 0 deletions docs/generated/sql/bnf/stmt_block.bnf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1871,6 +1871,7 @@ alter_table_cmd ::=
| 'ALTER' opt_column column_name alter_column_default
| 'ALTER' opt_column column_name 'DROP' 'NOT' 'NULL'
| 'ALTER' opt_column column_name 'DROP' 'STORED'
| 'ALTER' opt_column column_name 'SET' 'NOT' 'NULL'
| 'DROP' opt_column 'IF' 'EXISTS' column_name opt_drop_behavior
| 'DROP' opt_column column_name opt_drop_behavior
| 'ALTER' opt_column column_name opt_set_data 'TYPE' typename opt_collate opt_alter_column_using
Expand Down
6 changes: 0 additions & 6 deletions pkg/server/updates_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -478,11 +478,6 @@ func TestReportUsage(t *testing.T) {
) {
t.Fatal(err)
}
if _, err := db.Exec(`ALTER TABLE foo ALTER COLUMN x SET NOT NULL`); !testutils.IsError(
err, "unimplemented",
) {
t.Fatal(err)
}
if _, err := db.Exec(`SELECT crdb_internal.force_assertion_error('woo')`); !testutils.IsError(
err, "internal error",
) {
Expand Down Expand Up @@ -713,7 +708,6 @@ func TestReportUsage(t *testing.T) {

"unimplemented.#33285.json_object_agg": 10,
"unimplemented.pg_catalog.pg_stat_wal_receiver": 10,
"unimplemented.syntax.#28751": 10,
"unimplemented.syntax.#32564": 10,
"unimplemented.#9148": 10,
"othererror." +
Expand Down
34 changes: 34 additions & 0 deletions pkg/sql/alter_table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,41 @@ func applyColumnMutation(
}
}

case *tree.AlterTableSetNotNull:
if !col.Nullable {
return nil
}
// See if there's already a mutation to add a not null constraint
for i := range tableDesc.Mutations {
if constraint := tableDesc.Mutations[i].GetConstraint(); constraint != nil &&
constraint.ConstraintType == sqlbase.ConstraintToUpdate_NOT_NULL {
return nil
}
}

info, err := tableDesc.GetConstraintInfo(params.ctx, nil)
if err != nil {
return err
}
inuseNames := make(map[string]struct{}, len(info))
for k := range info {
inuseNames[k] = struct{}{}
}
tableDesc.AddNotNullValidationMutation(string(t.Column), col.ID, inuseNames)

case *tree.AlterTableDropNotNull:
if col.Nullable {
return nil
}
// See if there's already a mutation to add a not null constraint
for i := range tableDesc.Mutations {
if constraint := tableDesc.Mutations[i].GetConstraint(); constraint != nil &&
constraint.ConstraintType == sqlbase.ConstraintToUpdate_NOT_NULL {
return pgerror.Newf(pgcode.ObjectNotInPrerequisiteState,
"constraint in the middle of being added, try again later")
}
}
// TODO (lucy): As with FKs and check constraints, move this to the schema changer
col.Nullable = true

case *tree.AlterTableDropStored:
Expand Down
21 changes: 14 additions & 7 deletions pkg/sql/backfill.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func (sc *SchemaChanger) AddConstraints(
for i := range constraints {
added := &constraints[i]
switch added.ConstraintType {
case sqlbase.ConstraintToUpdate_CHECK:
case sqlbase.ConstraintToUpdate_CHECK, sqlbase.ConstraintToUpdate_NOT_NULL:
found := false
for _, c := range desc.Checks {
if c.Name == added.Name {
Expand Down Expand Up @@ -378,7 +378,7 @@ func (sc *SchemaChanger) validateConstraints(
// (the validation can take many minutes). So we pretend that the schema
// has been updated and actually update it in a separate transaction that
// follows this one.
desc, err := sqlbase.NewImmutableTableDescriptor(*tableDesc).MakeFirstMutationPublic()
desc, err := sqlbase.NewImmutableTableDescriptor(*tableDesc).MakeFirstMutationPublic(sqlbase.IgnoreConstraints)
if err != nil {
return err
}
Expand All @@ -387,13 +387,20 @@ func (sc *SchemaChanger) validateConstraints(
newEvalCtx := createSchemaChangeEvalCtx(ctx, readAsOf, evalCtx.Tracing, sc.ieFactory)
switch c.ConstraintType {
case sqlbase.ConstraintToUpdate_CHECK:
if err := validateCheckInTxn(ctx, sc.leaseMgr, &newEvalCtx.EvalContext, desc, txn, c.Name); err != nil {
if err := validateCheckInTxn(ctx, sc.leaseMgr, &newEvalCtx.EvalContext, desc, txn, c.Check.Name); err != nil {
return err
}
case sqlbase.ConstraintToUpdate_FOREIGN_KEY:
if err := validateFkInTxn(ctx, sc.leaseMgr, &newEvalCtx.EvalContext, desc, txn, c.Name); err != nil {
return err
}
case sqlbase.ConstraintToUpdate_NOT_NULL:
if err := validateCheckInTxn(ctx, sc.leaseMgr, &newEvalCtx.EvalContext, desc, txn, c.Check.Name); err != nil {
// TODO (lucy): This should distinguish between constraint
// validation errors and other types of unexpected errors, and
// return a different error code in the former case
return errors.Wrap(err, "validation of NOT NULL constraint failed")
}
default:
return errors.Errorf("unsupported constraint type: %d", c.ConstraintType)
}
Expand Down Expand Up @@ -1029,7 +1036,7 @@ func (sc *SchemaChanger) validateForwardIndexes(
// (the validation can take many minutes). So we pretend that the schema
// has been updated and actually update it in a separate transaction that
// follows this one.
desc, err := sqlbase.NewImmutableTableDescriptor(*tableDesc).MakeFirstMutationPublic()
desc, err := sqlbase.NewImmutableTableDescriptor(*tableDesc).MakeFirstMutationPublic(sqlbase.IgnoreConstraints)
if err != nil {
return err
}
Expand Down Expand Up @@ -1212,7 +1219,7 @@ func runSchemaChangesInTxn(

case *sqlbase.DescriptorMutation_Constraint:
switch t.Constraint.ConstraintType {
case sqlbase.ConstraintToUpdate_CHECK:
case sqlbase.ConstraintToUpdate_CHECK, sqlbase.ConstraintToUpdate_NOT_NULL:
tableDesc.Checks = append(tableDesc.Checks, &t.Constraint.Check)
case sqlbase.ConstraintToUpdate_FOREIGN_KEY:
idx, err := tableDesc.FindIndexByID(t.Constraint.ForeignKeyIndex)
Expand Down Expand Up @@ -1267,8 +1274,8 @@ func runSchemaChangesInTxn(
// mutations applied, it can be used for validating check constraints
for _, c := range constraintsToValidate {
switch c.ConstraintType {
case sqlbase.ConstraintToUpdate_CHECK:
if err := validateCheckInTxn(ctx, tc.leaseMgr, evalCtx, tableDesc, txn, c.Name); err != nil {
case sqlbase.ConstraintToUpdate_CHECK, sqlbase.ConstraintToUpdate_NOT_NULL:
if err := validateCheckInTxn(ctx, tc.leaseMgr, evalCtx, tableDesc, txn, c.Check.Name); err != nil {
return err
}
case sqlbase.ConstraintToUpdate_FOREIGN_KEY:
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/backfill/backfill.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ func ConvertBackfillError(
// information useful in printing a sensible error. However
// ConvertBatchError() will only work correctly if the schema elements
// are "live" in the tableDesc.
desc, err := tableDesc.MakeFirstMutationPublic()
desc, err := tableDesc.MakeFirstMutationPublic(sqlbase.IncludeConstraints)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/distsqlrun/indexbackfiller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (ib *indexBackfiller) wrapDupError(ctx context.Context, orig error) error {
return orig
}

desc, err := ib.desc.MakeFirstMutationPublic()
desc, err := ib.desc.MakeFirstMutationPublic(sqlbase.IncludeConstraints)
immutable := sqlbase.NewImmutableTableDescriptor(*desc.TableDesc())
if err != nil {
return err
Expand Down
58 changes: 58 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/alter_table
View file
Original file line number Diff line number Diff line change
Expand Up @@ -914,3 +914,61 @@ ALTER TABLE t32917_2 ADD CONSTRAINT fk_c_a FOREIGN KEY (c) references t32917 (a)

statement ok
ROLLBACK

# Test SET NOT NULL
statement ok
CREATE TABLE t (a INT)

statement ok
INSERT INTO t VALUES (1), (NULL)

statement error validation of NOT NULL constraint failed: validation of CHECK "a IS NOT NULL" failed
ALTER TABLE t ALTER COLUMN a SET NOT NULL

statement ok
DELETE FROM t WHERE a IS NULL

statement ok
ALTER TABLE t ALTER COLUMN a SET NOT NULL

statement error null value in column "a" violates not-null constraint
INSERT INTO t VALUES (NULL)

query TTTTB
SHOW CONSTRAINTS FROM t
----

statement ok
ALTER TABLE t ALTER COLUMN a DROP NOT NULL

statement ok
INSERT INTO t VALUES (NULL)

statement ok
DROP TABLE t

# Test interaction of SET NOT NULL with other constraints
statement ok
CREATE TABLE t (a INT)

statement ok
INSERT INTO t VALUES (1)

# Check for name collisions with the auto-generated NOT NULL check constraint name
statement ok
ALTER TABLE t ADD CONSTRAINT a_auto_not_null CHECK (a IS NOT NULL)

statement ok
ALTER TABLE t ADD CONSTRAINT a_auto_not_null1 CHECK (a IS NOT NULL), ALTER COLUMN a SET NOT NULL

statement error null value in column "a" violates not-null constraint
INSERT INTO t VALUES (NULL)

query TTTTB
SHOW CONSTRAINTS FROM t
----
t a_auto_not_null CHECK CHECK (a IS NOT NULL) true
t a_auto_not_null1 CHECK CHECK (a IS NOT NULL) true

statement ok
DROP TABLE t
Loading

0 comments on commit f6d75a5

Please sign in to comment.