diff --git a/pkg/ccl/importccl/read_import_mysql.go b/pkg/ccl/importccl/read_import_mysql.go index 9fb3c6ff34b6..0e7eaeec9365 100644 --- a/pkg/ccl/importccl/read_import_mysql.go +++ b/pkg/ccl/importccl/read_import_mysql.go @@ -494,7 +494,7 @@ type delayedFK struct { func addDelayedFKs(ctx context.Context, defs []delayedFK, resolver fkResolver) error { for _, def := range defs { if err := sql.ResolveFK( - ctx, nil, resolver, def.tbl, def.def, map[sqlbase.ID]*sqlbase.MutableTableDescriptor{}, sql.NewTable, + ctx, nil, resolver, def.tbl, def.def, map[sqlbase.ID]*sqlbase.MutableTableDescriptor{}, sql.NewTable, tree.ValidationDefault, ); err != nil { return err } diff --git a/pkg/ccl/importccl/read_import_pgdump.go b/pkg/ccl/importccl/read_import_pgdump.go index 46688470b877..6596419d7ad8 100644 --- a/pkg/ccl/importccl/read_import_pgdump.go +++ b/pkg/ccl/importccl/read_import_pgdump.go @@ -268,7 +268,7 @@ func readPostgresCreateTable( continue } for _, constraint := range constraints { - if err := sql.ResolveFK(evalCtx.Ctx(), nil /* txn */, fks.resolver, desc, constraint, backrefs, sql.NewTable); err != nil { + if err := sql.ResolveFK(evalCtx.Ctx(), nil /* txn */, fks.resolver, desc, constraint, backrefs, sql.NewTable, tree.ValidationDefault); err != nil { return nil, err } } diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index 9afdf8685fd0..d5195661a96e 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -273,7 +273,7 @@ func (n *alterTableNode) startExec(params runParams) error { } else { tableState = NonEmptyTable } - err = params.p.resolveFK(params.ctx, n.tableDesc, d, affected, tableState) + err = params.p.resolveFK(params.ctx, n.tableDesc, d, affected, tableState, t.ValidationBehavior) }) if err != nil { return err diff --git a/pkg/sql/create_table.go b/pkg/sql/create_table.go index 9f761ec9bf4d..3bd035b81593 100644 --- a/pkg/sql/create_table.go +++ b/pkg/sql/create_table.go @@ -384,8 +384,9 @@ func (p *planner) resolveFK( d *tree.ForeignKeyConstraintTableDef, backrefs map[sqlbase.ID]*sqlbase.MutableTableDescriptor, ts FKTableState, + validationBehavior tree.ValidationBehavior, ) error { - return ResolveFK(ctx, p.txn, p, tbl, d, backrefs, ts) + return ResolveFK(ctx, p.txn, p, tbl, d, backrefs, ts, validationBehavior) } func qualifyFKColErrorWithDB( @@ -421,8 +422,8 @@ const ( // It may, in doing so, add to or alter descriptors in the passed in `backrefs` // map of other tables that need to be updated when this table is created. // Constraints that are not known to hold for existing data are created -// "unvalidated", but when table is empty (e.g. during creation), no existing -// data imples no existing violations, and thus the constraint can be created +// "unvalidated", but when table is empty (e.g. during creating on), no existing +// data implies no existing violations, and thus the constraint can be created // without the unvalidated flag. // // The caller should pass an instance of fkSelfResolver as @@ -437,6 +438,10 @@ const ( // // The passed Txn is used to lookup databases to qualify names in error messages // but if nil, will result in unqualified names in those errors. +// +// The passed validationBehavior is used to determine whether or not preexisting +// entries in the table need to be validated against the foreign key being added. +// This only applies for existing tables, not new tables. func ResolveFK( ctx context.Context, txn *client.Txn, @@ -445,6 +450,7 @@ func ResolveFK( d *tree.ForeignKeyConstraintTableDef, backrefs map[sqlbase.ID]*sqlbase.MutableTableDescriptor, ts FKTableState, + validationBehavior tree.ValidationBehavior, ) error { for _, col := range d.FromCols { col, _, err := tbl.FindColumnByName(col) @@ -586,7 +592,11 @@ func ResolveFK( } if ts != NewTable { - ref.Validity = sqlbase.ConstraintValidity_Validating + if validationBehavior == tree.ValidationSkip { + ref.Validity = sqlbase.ConstraintValidity_Unvalidated + } else { + ref.Validity = sqlbase.ConstraintValidity_Validating + } } backref := sqlbase.ForeignKeyReference{Table: tbl.ID} @@ -1282,7 +1292,7 @@ func MakeTableDesc( desc.Checks = append(desc.Checks, ck) case *tree.ForeignKeyConstraintTableDef: - if err := ResolveFK(ctx, txn, fkResolver, &desc, d, affected, NewTable); err != nil { + if err := ResolveFK(ctx, txn, fkResolver, &desc, d, affected, NewTable, tree.ValidationDefault); err != nil { return desc, err } diff --git a/pkg/sql/logictest/testdata/logic_test/fk b/pkg/sql/logictest/testdata/logic_test/fk index 2952c2193fd8..f7000727d76f 100644 --- a/pkg/sql/logictest/testdata/logic_test/fk +++ b/pkg/sql/logictest/testdata/logic_test/fk @@ -1390,10 +1390,6 @@ subtest unvalidated_fk_plan # To get an unvalidated foreign key for testing, use the loophole that we # currently don't support adding a validated FK in the same transaction as # CREATE TABLE -# TODO (lucy): Once this is no longer true (and we support adding NOT VALID -# constraints), update this test -statement ok -BEGIN statement ok CREATE TABLE a ( @@ -1415,10 +1411,10 @@ statement ok INSERT INTO b (a_x, a_y, a_z) VALUES ('x2', 'y1', 'z1') statement ok -ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) NOT VALID -statement ok -COMMIT +statement error pq: foreign key violation: "b" row a_z='z1', a_y='y1', a_x='x2', rowid=[0-9]* has no match in "a" +ALTER TABLE b VALIDATE CONSTRAINT fk_ref # Verify that the optimizer doesn't use an unvalidated constraint to simplify plans. query TTT @@ -1691,8 +1687,8 @@ ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, statement ok DROP TABLE b, a -subtest Composite_Full -# Originally from 26748. +subtest Composite_Simple_Unvalidated +# Test inserting into table with an unvalidated constraint, and running VALIDATE CONSTRAINT later # Test composite key with two columns. statement ok @@ -1704,34 +1700,45 @@ CREATE TABLE a ( statement ok CREATE TABLE b ( - a_y STRING NULL + a_y STRING NULL ,a_x STRING NULL - ,CONSTRAINT fk_ref FOREIGN KEY (a_y, a_x) REFERENCES a (y, x) MATCH FULL ); +# Add the constraint separately so that it's unvalidated, so we can test VALIDATE CONSTRAINT. +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_y, a_x) REFERENCES a (y, x) + statement ok INSERT INTO a (x, y) VALUES ('x1', 'y1') -# These statements should all fail because this uses MATCH FULL. -statement error missing value for column "a_y" in multi-part foreign key +# All of these are allowed because we do composite matching using MATCH SIMPLE. +statement ok INSERT INTO b (a_x) VALUES ('x1') -statement error missing value for column "a_x" in multi-part foreign key +statement ok INSERT INTO b (a_y) VALUES ('y1') -statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +statement ok INSERT INTO b (a_y, a_x) VALUES ('y1', NULL) -statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +statement ok INSERT INTO b (a_y, a_x) VALUES (NULL, 'x1') -# These next two statements should still be allowed. +statement ok +INSERT INTO b (a_y, a_x) VALUES ('y2', NULL) + +statement ok +INSERT INTO b (a_y, a_x) VALUES (NULL, 'x2') + statement ok INSERT INTO b (a_x, a_y) VALUES ('x1', 'y1') statement ok INSERT INTO b (a_x, a_y) VALUES (NULL, NULL) +statement ok +ALTER TABLE b VALIDATE CONSTRAINT fk_ref + statement ok DROP TABLE b, a @@ -1749,9 +1756,165 @@ CREATE TABLE b ( a_y STRING NULL ,a_x STRING NULL ,a_z STRING NULL - ,CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) MATCH FULL ); +# Add the constraint separately so that it's unvalidated, so we can test VALIDATE CONSTRAINT. +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) + +statement ok +INSERT INTO a (x, y, z) VALUES ('x1', 'y1', 'z1') + +# All of these are allowed because we do composite matching using MATCH SIMPLE. +statement ok +INSERT INTO b (a_x) VALUES ('x1') + +statement ok +INSERT INTO b (a_y) VALUES ('y1') + +statement ok +INSERT INTO b (a_z) VALUES ('z1') + +statement ok +INSERT INTO b (a_x, a_y) VALUES ('x1', 'y1') + +statement ok +INSERT INTO b (a_x, a_y) VALUES (NULL, 'y1') + +statement ok +INSERT INTO b (a_x, a_y) VALUES ('x1', NULL) + +statement ok +INSERT INTO b (a_x, a_z) VALUES ('x1', 'z1') + +statement ok +INSERT INTO b (a_x, a_z) VALUES (NULL, 'z1') + +statement ok +INSERT INTO b (a_x, a_z) VALUES ('x1', NULL) + +statement ok +INSERT INTO b (a_y, a_z) VALUES ('y1', 'z1') + +statement ok +INSERT INTO b (a_y, a_z) VALUES (NULL, 'z1') + +statement ok +INSERT INTO b (a_y, a_z) VALUES ('y1', NULL) + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES ('x1', NULL, NULL) + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES (NULL, 'y1', NULL) + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES (NULL, NULL, 'z1') + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES ('x1', 'y1', NULL) + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES ('x1', NULL, 'z1') + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES (NULL, 'y1', 'z1') + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES ('x2', NULL, NULL) + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES (NULL, 'y2', NULL) + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES (NULL, NULL, 'z2') + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES ('x2', 'y2', NULL) + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES ('x2', NULL, 'z2') + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES (NULL, 'y2', 'z2') + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES (NULL, NULL, NULL) + +statement ok +ALTER TABLE b VALIDATE CONSTRAINT fk_ref + +statement ok +DROP TABLE b, a + +#subtest Composite_Simple_Validate_Constraint_Invalid + +subtest Composite_Full +# Originally from 26748. + +# Test composite key with two columns. +statement ok +CREATE TABLE a ( + x STRING NULL, + y STRING NULL, + CONSTRAINT "primary" PRIMARY KEY (y, x) +); + +statement ok +CREATE TABLE b ( + a_y STRING NULL, + a_x STRING NULL +); + +# Add the constraint separately so that it's unvalidated, so we can test VALIDATE CONSTRAINT. +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_y, a_x) REFERENCES a (y, x) MATCH FULL NOT VALID + +statement ok +INSERT INTO a (x, y) VALUES ('x1', 'y1') + +# These statements should all fail because this uses MATCH FULL. +statement error missing value for column "a_y" in multi-part foreign key +INSERT INTO b (a_x) VALUES ('x1') + +statement error missing value for column "a_x" in multi-part foreign key +INSERT INTO b (a_y) VALUES ('y1') + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +INSERT INTO b (a_y, a_x) VALUES ('y1', NULL) + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +INSERT INTO b (a_y, a_x) VALUES (NULL, 'x1') + +# These next two statements should still be allowed. +statement ok +INSERT INTO b (a_x, a_y) VALUES ('x1', 'y1') + +statement ok +INSERT INTO b (a_x, a_y) VALUES (NULL, NULL) + +statement ok +DROP TABLE b, a + +# Test composite key with three columns. +statement ok +CREATE TABLE a ( + x STRING NULL, + y STRING NULL, + z STRING NULL, + CONSTRAINT "primary" PRIMARY KEY (z, y, x) +); + +statement ok +CREATE TABLE b ( + a_y STRING NULL, + a_x STRING NULL, + a_z STRING NULL +); + +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) MATCH FULL NOT VALID + statement ok INSERT INTO a (x, y, z) VALUES ('x1', 'y1', 'z1') @@ -1949,6 +2112,295 @@ ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, statement ok DROP TABLE b, a +subtest Composite_Full_Validate_Later +# Test inserting into table with an unvalidated constraint, and running VALIDATE CONSTRAINT later + +# Test composite key with two columns. +statement ok +CREATE TABLE a ( + x STRING NULL + ,y STRING NULL + ,CONSTRAINT "primary" PRIMARY KEY (y, x) +); + +statement ok +CREATE TABLE b ( + a_y STRING NULL + ,a_x STRING NULL +); + +# Add the constraint separately so that it's unvalidated, so we can test VALIDATE CONSTRAINT. +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_y, a_x) REFERENCES a (y, x) MATCH FULL NOT VALID + +statement ok +INSERT INTO a (x, y) VALUES ('x1', 'y1') + +# These statements should all fail because this uses MATCH FULL. +statement error missing value for column "a_y" in multi-part foreign key +INSERT INTO b (a_x) VALUES ('x1') + +statement error missing value for column "a_x" in multi-part foreign key +INSERT INTO b (a_y) VALUES ('y1') + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +INSERT INTO b (a_y, a_x) VALUES ('y1', NULL) + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +INSERT INTO b (a_y, a_x) VALUES (NULL, 'x1') + +# These next two statements should still be allowed. +statement ok +INSERT INTO b (a_x, a_y) VALUES ('x1', 'y1') + +statement ok +INSERT INTO b (a_x, a_y) VALUES (NULL, NULL) + +statement ok +ALTER TABLE b VALIDATE CONSTRAINT fk_ref + +statement ok +DROP TABLE b, a + +# Test composite key with three columns. +statement ok +CREATE TABLE a ( + x STRING NULL + ,y STRING NULL + ,z STRING NULL + ,CONSTRAINT "primary" PRIMARY KEY (z, y, x) +); + +statement ok +CREATE TABLE b ( + a_y STRING NULL + ,a_x STRING NULL + ,a_z STRING NULL +); + +# Add the constraint separately so that it's unvalidated, so we can test VALIDATE CONSTRAINT. +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) MATCH FULL NOT VALID + +statement ok +INSERT INTO a (x, y, z) VALUES ('x1', 'y1', 'z1') + +# These statements should all fail because this uses MATCH FULL. +statement error missing values for columns \["a_y" "a_z"\] in multi-part foreign key +INSERT INTO b (a_x) VALUES ('x1') + +statement error missing values for columns \["a_x" "a_z"\] in multi-part foreign key +INSERT INTO b (a_y) VALUES ('y1') + +statement error missing values for columns \["a_x" "a_y"\] in multi-part foreign key +INSERT INTO b (a_z) VALUES ('z1') + +statement error missing value for column "a_z" in multi-part foreign key +INSERT INTO b (a_x, a_y) VALUES ('x1', 'y1') + +statement error missing value for column "a_z" in multi-part foreign key +INSERT INTO b (a_x, a_y) VALUES (NULL, 'y1') + +statement error missing value for column "a_z" in multi-part foreign key +INSERT INTO b (a_x, a_y) VALUES ('x1', NULL) + +statement error missing value for column "a_y" in multi-part foreign key +INSERT INTO b (a_x, a_z) VALUES ('x1', 'z1') + +statement error missing value for column "a_y" in multi-part foreign key +INSERT INTO b (a_x, a_z) VALUES (NULL, 'z1') + +statement error missing value for column "a_y" in multi-part foreign key +INSERT INTO b (a_x, a_z) VALUES ('x1', NULL) + +statement error missing value for column "a_x" in multi-part foreign key +INSERT INTO b (a_y, a_z) VALUES ('y1', 'z1') + +statement error missing value for column "a_x" in multi-part foreign key +INSERT INTO b (a_y, a_z) VALUES (NULL, 'z1') + +statement error missing value for column "a_x" in multi-part foreign key +INSERT INTO b (a_y, a_z) VALUES ('y1', NULL) + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +INSERT INTO b (a_x, a_y, a_z) VALUES ('x1', NULL, NULL) + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +INSERT INTO b (a_x, a_y, a_z) VALUES (NULL, 'y1', NULL) + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +INSERT INTO b (a_x, a_y, a_z) VALUES (NULL, NULL, 'z1') + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +INSERT INTO b (a_x, a_y, a_z) VALUES ('x1', 'y1', NULL) + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +INSERT INTO b (a_x, a_y, a_z) VALUES ('x1', NULL, 'z1') + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +INSERT INTO b (a_x, a_y, a_z) VALUES (NULL, 'y1', 'z1') + +# This statement should still be allowed. +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES (NULL, NULL, NULL) + +statement ok +ALTER TABLE b VALIDATE CONSTRAINT fk_ref + +statement ok +DROP TABLE b, a + +subtest Composite_Full_Validate_Constraint_Invalid +# Test VALIDATE CONSTRAINT by inserting invalid rows before the constraint is added, one at a time. + +statement ok +CREATE TABLE a ( + x STRING NULL + ,y STRING NULL + ,z STRING NULL + ,CONSTRAINT "primary" PRIMARY KEY (z, y, x) +); + +statement ok +CREATE TABLE b ( + a_y STRING NULL + ,a_x STRING NULL + ,a_z STRING NULL + ,INDEX idx (a_z, a_y, a_x) +); + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES ('x1', NULL, NULL) + +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) MATCH FULL NOT VALID + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +ALTER TABLE b VALIDATE CONSTRAINT fk_ref + +statement ok +TRUNCATE b + +statement ok +ALTER TABLE b DROP CONSTRAINT fk_ref + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES (NULL, 'y1', NULL) + +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) MATCH FULL NOT VALID + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +ALTER TABLE b VALIDATE CONSTRAINT fk_ref + +statement ok +TRUNCATE b + +statement ok +ALTER TABLE b DROP CONSTRAINT fk_ref + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES (NULL, NULL, 'z1') + +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) MATCH FULL NOT VALID + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +ALTER TABLE b VALIDATE CONSTRAINT fk_ref + +statement ok +TRUNCATE b + +statement ok +ALTER TABLE b DROP CONSTRAINT fk_ref + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES ('x1', 'y1', NULL) + +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) MATCH FULL NOT VALID + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +ALTER TABLE b VALIDATE CONSTRAINT fk_ref + +statement ok +TRUNCATE b + +statement ok +ALTER TABLE b DROP CONSTRAINT fk_ref + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES ('x1', NULL, 'z1') + +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) MATCH FULL NOT VALID + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +ALTER TABLE b VALIDATE CONSTRAINT fk_ref + +statement ok +TRUNCATE b + +statement ok +ALTER TABLE b DROP CONSTRAINT fk_ref + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES (NULL, 'y1', 'z1') + +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) MATCH FULL NOT VALID + +statement error foreign key violation: MATCH FULL does not allow mixing of null and nonnull values +ALTER TABLE b VALIDATE CONSTRAINT fk_ref + +statement ok +TRUNCATE b + +statement ok +ALTER TABLE b DROP CONSTRAINT fk_ref + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES ('x2', 'y1', 'z1') + +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) MATCH FULL NOT VALID + +statement error pq: foreign key violation: "b" row a_z='z1', a_y='y1', a_x='x2', rowid=[0-9]* has no match in "a" +ALTER TABLE b VALIDATE CONSTRAINT fk_ref + +statement ok +TRUNCATE b + +statement ok +ALTER TABLE b DROP CONSTRAINT fk_ref + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES ('x2', 'y2', 'z1') + +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) MATCH FULL NOT VALID + +statement error pq: foreign key violation: "b" row a_z='z1', a_y='y2', a_x='x2', rowid=[0-9]* has no match in "a" +ALTER TABLE b VALIDATE CONSTRAINT fk_ref + +statement ok +TRUNCATE b + +statement ok +ALTER TABLE b DROP CONSTRAINT fk_ref + +statement ok +INSERT INTO b (a_x, a_y, a_z) VALUES ('x2', 'y2', 'z2') + +statement ok +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) MATCH FULL NOT VALID + +statement error pq: foreign key violation: "b" row a_z='z2', a_y='y2', a_x='x2', rowid=[0-9]* has no match in "a" +ALTER TABLE b VALIDATE CONSTRAINT fk_ref + +statement ok +DROP TABLE b, a + subtest auto_add_fk_with_composite_index_to_empty_table statement ok @@ -2101,3 +2553,44 @@ DELETE FROM t1 WHERE x IS NULL statement ok DROP TABLE t2, t2 CASCADE + +subtest test_not_valid_fk + +statement ok +CREATE TABLE person (id INT PRIMARY KEY, age INT, name STRING) + +statement ok +CREATE TABLE pet (id INT PRIMARY KEY, name STRING) + +statement ok +INSERT INTO pet VALUES (0, 'crookshanks') + +statement error pq: foreign key violation: "pet" row id=0 has no match in "person" +ALTER TABLE pet ADD CONSTRAINT fk_constraint FOREIGN KEY (id) REFERENCES person (id) + +statement ok +ALTER TABLE pet ADD CONSTRAINT fk_constraint FOREIGN KEY (id) REFERENCES person (id) NOT VALID + +query TTTTB +SHOW CONSTRAINTS FROM pet +---- +pet fk_constraint FOREIGN KEY FOREIGN KEY (id) REFERENCES person(id) false +pet primary PRIMARY KEY PRIMARY KEY (id ASC) true + +statement error pq: foreign key violation: "pet" row id=0 has no match in "person" +ALTER TABLE pet VALIDATE CONSTRAINT fk_constraint + +statement ok +INSERT INTO person VALUES (0, 18, 'Hermione Granger') + +statement ok +ALTER TABLE pet VALIDATE CONSTRAINT fk_constraint + +query TTTTB +SHOW CONSTRAINTS FROM pet +---- +pet fk_constraint FOREIGN KEY FOREIGN KEY (id) REFERENCES person(id) true +pet primary PRIMARY KEY PRIMARY KEY (id ASC) true + +statement ok +DROP TABLE person, pet 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 b97dd7eaa036..53f21ba50124 100644 --- a/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn +++ b/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn @@ -934,7 +934,7 @@ statement ok ALTER TABLE check_table ADD c INT statement ok -ALTER TABLE check_table ADD CONSTRAINT c_0 CHECK (c > 0) +ALTER TABLE check_table ADD CONSTRAINT c_0 CHECK (c > 0) NOT VALID statement ok ALTER TABLE check_table ADD d INT DEFAULT 1 @@ -948,7 +948,7 @@ COMMIT query TTTTB SHOW CONSTRAINTS FROM check_table ---- -check_table c_0 CHECK CHECK ((c > 0)) true +check_table c_0 CHECK CHECK ((c > 0)) false check_table d_0 CHECK CHECK ((d > 0)) true check_table primary PRIMARY KEY PRIMARY KEY (k ASC) true @@ -981,7 +981,7 @@ COMMIT query TTTTB SHOW CONSTRAINTS FROM check_table ---- -check_table c_0 CHECK CHECK ((c > 0)) true +check_table c_0 CHECK CHECK ((c > 0)) false check_table d_0 CHECK CHECK ((d > 0)) true check_table primary PRIMARY KEY PRIMARY KEY (k ASC) true diff --git a/pkg/sql/opt/exec/execbuilder/testdata/fk b/pkg/sql/opt/exec/execbuilder/testdata/fk index 79b0e23ff67f..c8d386b3ae8c 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/fk +++ b/pkg/sql/opt/exec/execbuilder/testdata/fk @@ -3,10 +3,6 @@ # To get an unvalidated foreign key for testing, use the loophole that we # currently don't support adding a validated FK in the same transaction as # CREATE TABLE -# TODO (lucy): Once this is no longer true (and we support adding NOT VALID -# constraints), update this test -statement ok -BEGIN statement ok CREATE TABLE a ( @@ -25,10 +21,7 @@ CREATE TABLE b ( ) statement ok -ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) - -statement ok -COMMIT +ALTER TABLE b ADD CONSTRAINT fk_ref FOREIGN KEY (a_z, a_y, a_x) REFERENCES a (z, y, x) NOT VALID # Verify that the optimizer doesn't use an unvalidated constraint to simplify plans. query TTT colnames diff --git a/pkg/sql/schema_changer.go b/pkg/sql/schema_changer.go index d522d1ef4538..ec8cbce26f8b 100644 --- a/pkg/sql/schema_changer.go +++ b/pkg/sql/schema_changer.go @@ -1210,13 +1210,54 @@ func (sc *SchemaChanger) done(ctx context.Context) (*sqlbase.ImmutableTableDescr isRollback := false jobSucceeded := true now := timeutil.Now().UnixNano() - return sc.leaseMgr.Publish(ctx, sc.tableID, func(desc *sqlbase.MutableTableDescriptor) error { + + // Get the other tables whose foreign key backreferences need to be removed. + // We make a call to PublishMultiple to handle the situation to add Foreign Key backreferences. + var fksByBackrefTable map[sqlbase.ID][]*sqlbase.ConstraintToUpdate + err := sc.db.Txn(ctx, func(ctx context.Context, txn *client.Txn) error { + fksByBackrefTable = make(map[sqlbase.ID][]*sqlbase.ConstraintToUpdate) + + desc, err := sqlbase.GetTableDescFromID(ctx, txn, sc.tableID) + if err != nil { + return err + } + for _, mutation := range desc.Mutations { + if mutation.MutationID != sc.mutationID { + break + } + if constraint := mutation.GetConstraint(); constraint != nil && + constraint.ConstraintType == sqlbase.ConstraintToUpdate_FOREIGN_KEY && + mutation.Direction == sqlbase.DescriptorMutation_ADD && + constraint.ForeignKey.Validity == sqlbase.ConstraintValidity_Unvalidated { + // Add backref table to referenced table with an unvalidated foreign key constraint + fk := &constraint.ForeignKey + if fk.Table != desc.ID { + fksByBackrefTable[constraint.ForeignKey.Table] = append(fksByBackrefTable[constraint.ForeignKey.Table], constraint) + } + } + } + return nil + }) + if err != nil { + return nil, err + } + tableIDsToUpdate := make([]sqlbase.ID, 0, len(fksByBackrefTable)+1) + tableIDsToUpdate = append(tableIDsToUpdate, sc.tableID) + for id := range fksByBackrefTable { + tableIDsToUpdate = append(tableIDsToUpdate, id) + } + + update := func(descs map[sqlbase.ID]*sqlbase.MutableTableDescriptor) error { // Reset vars here because update function can be called multiple times in a retry. isRollback = false jobSucceeded = true i := 0 - for _, mutation := range desc.Mutations { + scDesc, ok := descs[sc.tableID] + if !ok { + return errors.AssertionFailedf("required table with ID %d not provided to update closure", sc.tableID) + } + for _, mutation := range scDesc.Mutations { if mutation.MutationID != sc.mutationID { // Mutations are applied in a FIFO order. Only apply the first set of // mutations if they have the mutation ID we're looking for. @@ -1227,8 +1268,8 @@ func (sc *SchemaChanger) done(ctx context.Context) (*sqlbase.ImmutableTableDescr indexDesc != nil { if sc.canClearRangeForDrop(indexDesc) { jobSucceeded = false - desc.GCMutations = append( - desc.GCMutations, + scDesc.GCMutations = append( + scDesc.GCMutations, sqlbase.TableDescriptor_GCDescriptorMutation{ IndexID: indexDesc.ID, DropTime: now, @@ -1236,7 +1277,23 @@ func (sc *SchemaChanger) done(ctx context.Context) (*sqlbase.ImmutableTableDescr }) } } - if err := desc.MakeMutationComplete(mutation); err != nil { + if constraint := mutation.GetConstraint(); constraint != nil && + constraint.ConstraintType == sqlbase.ConstraintToUpdate_FOREIGN_KEY && + mutation.Direction == sqlbase.DescriptorMutation_ADD && + constraint.ForeignKey.Validity == sqlbase.ConstraintValidity_Unvalidated { + // Add backreference on the referenced table (which could be the same table) + backref := sqlbase.ForeignKeyReference{Table: sc.tableID, Index: constraint.ForeignKeyIndex} + backrefTable, ok := descs[constraint.ForeignKey.Table] + if !ok { + return errors.AssertionFailedf("required table with ID %d not provided to update closure", sc.tableID) + } + backrefIdx, err := backrefTable.FindIndexByID(constraint.ForeignKey.Index) + if err != nil { + return err + } + backrefIdx.ReferencedBy = append(backrefIdx.ReferencedBy, backref) + } + if err := scDesc.MakeMutationComplete(mutation); err != nil { return err } i++ @@ -1247,17 +1304,19 @@ func (sc *SchemaChanger) done(ctx context.Context) (*sqlbase.ImmutableTableDescr return errDidntUpdateDescriptor } // Trim the executed mutations from the descriptor. - desc.Mutations = desc.Mutations[i:] + scDesc.Mutations = scDesc.Mutations[i:] - for i, g := range desc.MutationJobs { + for i, g := range scDesc.MutationJobs { if g.MutationID == sc.mutationID { // Trim the executed mutation group from the descriptor. - desc.MutationJobs = append(desc.MutationJobs[:i], desc.MutationJobs[i+1:]...) + scDesc.MutationJobs = append(scDesc.MutationJobs[:i], scDesc.MutationJobs[i+1:]...) break } } return nil - }, func(txn *client.Txn) error { + } + + descs, err := sc.leaseMgr.PublishMultiple(ctx, tableIDsToUpdate, update, func(txn *client.Txn) error { if jobSucceeded { if err := sc.job.WithTxn(txn).Succeeded(ctx, jobs.NoopFn); err != nil { return errors.Wrapf(err, @@ -1292,6 +1351,10 @@ func (sc *SchemaChanger) done(ctx context.Context) (*sqlbase.ImmutableTableDescr }{uint32(sc.mutationID)}, ) }) + if err != nil { + return nil, err + } + return descs[sc.tableID], nil } // notFirstInLine returns true whenever the schema change has been queued @@ -1353,7 +1416,7 @@ func (sc *SchemaChanger) refreshStats() { // reverseMutations reverses the direction of all the mutations with the // mutationID. This is called after hitting an irrecoverable error while -// applying a schema change. If a column being added is reversed and droped, +// applying a schema change. If a column being added is reversed and dropped, // all new indexes referencing the column will also be dropped. func (sc *SchemaChanger) reverseMutations(ctx context.Context, causingError error) error { // Reverse the flow of the state machine. @@ -1374,7 +1437,8 @@ func (sc *SchemaChanger) reverseMutations(ctx context.Context, causingError erro } if constraint := mutation.GetConstraint(); constraint != nil && constraint.ConstraintType == sqlbase.ConstraintToUpdate_FOREIGN_KEY && - mutation.Direction == sqlbase.DescriptorMutation_ADD { + mutation.Direction == sqlbase.DescriptorMutation_ADD && + constraint.ForeignKey.Validity == sqlbase.ConstraintValidity_Validating { fk := &constraint.ForeignKey if fk.Table != desc.ID { fksByBackrefTable[constraint.ForeignKey.Table] = append(fksByBackrefTable[constraint.ForeignKey.Table], constraint) diff --git a/pkg/sql/sqlbase/structured.go b/pkg/sql/sqlbase/structured.go index 4246f18af218..eeb3e877986f 100644 --- a/pkg/sql/sqlbase/structured.go +++ b/pkg/sql/sqlbase/structured.go @@ -2329,8 +2329,8 @@ func (desc *TableDescriptor) FindIndexByID(id IndexID) (*IndexDescriptor, error) // FindIndexByIndexIdx returns an active index with the specified // index's index which has a domain of [0, # of secondary indexes] and whether // the index is a secondary index. -// The primary index has an index of 0 and the first secondary index (if it exists) -// has an index of 1. +// The primary index has an index of 0 and the first secondary index +// (if it exists) has an index of 1. func (desc *TableDescriptor) FindIndexByIndexIdx( indexIdx int, ) (index *IndexDescriptor, isSecondary bool, err error) { @@ -2374,6 +2374,14 @@ func (desc *TableDescriptor) IsInterleaved() bool { } // MakeMutationComplete updates the descriptor upon completion of a mutation. +// There are three Validity types for the mutations: +// Validated - The constraint has already been added and validated, should +// never be the case for a validated constraint to enter this +// method. +// Validating - The constraint has already been added, and just needs to be +// marked as validated. +// Unvalidated - The constraint has not yet been added, and needs to be added +// for the first time. func (desc *MutableTableDescriptor) MakeMutationComplete(m DescriptorMutation) error { switch m.Direction { case DescriptorMutation_ADD: @@ -2390,15 +2398,18 @@ func (desc *MutableTableDescriptor) MakeMutationComplete(m DescriptorMutation) e switch t.Constraint.ConstraintType { case ConstraintToUpdate_CHECK: switch t.Constraint.Check.Validity { - case ConstraintValidity_Unvalidated: - desc.Checks = append(desc.Checks, &t.Constraint.Check) case ConstraintValidity_Validating: + // Constraint already added, just mark it as Validated for _, c := range desc.Checks { if c.Name == t.Constraint.Name { c.Validity = ConstraintValidity_Validated break } } + case ConstraintValidity_Unvalidated: + // add the constraint to the list of check constraints on the table + // descriptor + desc.Checks = append(desc.Checks, &t.Constraint.Check) default: return errors.AssertionFailedf("invalid constraint validity state: %d", t.Constraint.Check.Validity) } @@ -2407,7 +2418,17 @@ func (desc *MutableTableDescriptor) MakeMutationComplete(m DescriptorMutation) e if err != nil { return err } - idx.ForeignKey.Validity = ConstraintValidity_Validated + switch t.Constraint.ForeignKey.Validity { + case ConstraintValidity_Validating: + // Constraint already added, just mark it as Validated + idx.ForeignKey.Validity = ConstraintValidity_Validated + case ConstraintValidity_Unvalidated: + // 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. + // TODO (tyler): Combine both of these tasks in the same place. + idx.ForeignKey = t.Constraint.ForeignKey + } case ConstraintToUpdate_NOT_NULL: // Remove the dummy check constraint that was in place during validation for i, c := range desc.Checks {