Bug fix: Allow unresolved FKs to merge with resolved FKs

go/libraries/doltcore/doltdb/foreign_key_coll.go

@@ -470,25 +470,33 @@ OuterLoop:
 }
 
 // GetMatchingKey gets the ForeignKey defined over the parent and child columns. If the given foreign key is resolved,
-// then both resolved and unresolved keys are checked for a match. If the given foreign key is unresolved, then ONLY
-// unresolved keys may be found.
+// then both resolved and unresolved keys are checked for a match. If the given foreign key is unresolved, then it
+// will match with unresolved keys, and may also match with resolved keys if |matchUnresolvedKeyToResolvedKey| is
+// set to true.
 //
-// This discrepancy is due to the primary uses for this function. It is assumed that the ForeignKeyCollection is an
+// When |matchUnresolvedKeyToResolvedKey| is false, it is assumed that the ForeignKeyCollection is an
 // ancestor collection compared to the collection that the given key comes from. Therefore, the key found in the
 // ancestor will usually be the unresolved version of the given key, hence the comparison is valid. However, if the
 // given key is unresolved, it is treated as a new key, which cannot be matched to a resolved key that previously
 // existed.
 //
-// The given schema map is keyed by table name, and is used in the event that the given key is resolved and any keys in
-// the collection are unresolved. A "dirty resolution" is performed, which matches the column names to tags, and then a
-// standard tag comparison is performed. If a table or column is not in the map, then the foreign key is ignored.
-func (fkc *ForeignKeyCollection) GetMatchingKey(fk ForeignKey, allSchemas map[string]schema.Schema) (ForeignKey, bool) {
+// When the ForeignKeyCollection is NOT an ancestor collection, |matchUnresolvedKeyToResolvedKey| can be set to true
+// to enable |fk| to match against both resolved and unresolved keys in the ForeignKeyCollection. This is necessary
+// when one session commits a table with resolved foreign keys, but another open session still has an unresolved
+// version of the foreign key. When the open session commits, it needs to merge it's root with the root that has
+// the resolved foreign key, so it's unresolved foreign key needs to be able to match a resolved key in this case.
+//
+// The given schema map, |allSchemas|, is keyed by table name, and is used in the event that the given key is resolved
+// and any keys in the collection are unresolved. A "dirty resolution" is performed, which matches the column names to
+// tags, and then a standard tag comparison is performed. If a table or column is not in the map, then the foreign key
+// is ignored.
+func (fkc *ForeignKeyCollection) GetMatchingKey(fk ForeignKey, allSchemas map[string]schema.Schema, matchUnresolvedKeyToResolvedKey bool) (ForeignKey, bool) {
 	if !fk.IsResolved() {
 		// The given foreign key is unresolved, so we only look for matches on unresolved keys
 	OuterLoopUnresolved:
 		for _, existingFk := range fkc.foreignKeys {
 			// For unresolved keys, the table name is important (column tags are globally unique, column names are not)
-			if existingFk.IsResolved() ||
+			if (!matchUnresolvedKeyToResolvedKey && existingFk.IsResolved()) ||
 				fk.TableName != existingFk.TableName ||
 				fk.ReferencedTableName != existingFk.ReferencedTableName ||
 				len(fk.UnresolvedFKDetails.TableColumns) != len(existingFk.UnresolvedFKDetails.TableColumns) ||

go/libraries/doltcore/merge/merge_schema.go

@@ -909,7 +909,11 @@ func foreignKeysInCommon(ourFKs, theirFKs, ancFKs *doltdb.ForeignKeyCollection,
 	common, _ = doltdb.NewForeignKeyCollection()
 	err = ourFKs.Iter(func(ours doltdb.ForeignKey) (stop bool, err error) {
 
-		theirs, ok := theirFKs.GetMatchingKey(ours, ancSchs)
+		// Since we aren't using an ancestor root here, pass true for the
+		// matchUnresolvedKeyToResolvedKey parameter. This allows us to match
+		// resolved FKs with both resolved and unresolved FKs in theirFKs.
+		// See GetMatchingKey's documentation for more info.
+		theirs, ok := theirFKs.GetMatchingKey(ours, ancSchs, true)
 		if !ok {
 			return false, nil
 		}
@@ -919,7 +923,7 @@ func foreignKeysInCommon(ourFKs, theirFKs, ancFKs *doltdb.ForeignKeyCollection,
 			return false, err
 		}
 
-		anc, ok := ancFKs.GetMatchingKey(ours, ancSchs)
+		anc, ok := ancFKs.GetMatchingKey(ours, ancSchs, false)
 		if !ok {
 			// FKs added on both branch with different defs
 			conflicts = append(conflicts, FKConflict{
@@ -981,7 +985,7 @@ func foreignKeysInCommon(ourFKs, theirFKs, ancFKs *doltdb.ForeignKeyCollection,
 func fkCollSetDifference(fkColl, ancestorFkColl *doltdb.ForeignKeyCollection, ancSchs map[string]schema.Schema) (d *doltdb.ForeignKeyCollection, err error) {
 	d, _ = doltdb.NewForeignKeyCollection()
 	err = fkColl.Iter(func(fk doltdb.ForeignKey) (stop bool, err error) {
-		_, ok := ancestorFkColl.GetMatchingKey(fk, ancSchs)
+		_, ok := ancestorFkColl.GetMatchingKey(fk, ancSchs, false)
 		if !ok {
 			err = d.AddKeys(fk)
 		}

go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go

@@ -755,6 +755,64 @@ var DoltTransactionTests = []queries.TransactionTest{
 			},
 		},
 	},
+	{
+		// https://github.com/dolthub/dolt/issues/7956
+		Name: "Merge unresolved FKs after resolved FKs were committed",
+		Assertions: []queries.ScriptTestAssertion{
+			{
+				Query:            "/* client a */ SET @@foreign_key_checks=0;",
+				SkipResultsCheck: true,
+			},
+			{
+				Query:            "/* client a */ SET @@autocommit=0;",
+				SkipResultsCheck: true,
+			},
+			{
+				// Create a table for the FK to reference
+				Query:    "/* client a */ CREATE TABLE ref (id varchar(100) PRIMARY KEY, status int);",
+				Expected: []sql.Row{{types.NewOkResult(0)}},
+			},
+			{
+				// Create a table with an FK
+				Query:    "/* client a */ CREATE TABLE t (id int, ref_id varchar(100), FOREIGN KEY (ref_id) REFERENCES ref(id));",
+				Expected: []sql.Row{{types.NewOkResult(0)}},
+			},
+			{
+				Query:    "/* client a */ COMMIT;",
+				Expected: []sql.Row{},
+			},
+			{
+				// Turn @@foreign_key_checks back on in client a
+				Query:    "/* client a */ SET @@foreign_key_checks=1;",
+				Expected: []sql.Row{{}},
+			},
+			{
+				// Reference the table with an unresolved FK, so that it gets loaded and resolved
+				Query:    "/* client a */ UPDATE t SET ref_id = 42 where ref_id > 100000;",
+				Expected: []sql.Row{{types.OkResult{Info: plan.UpdateInfo{}}}},
+			},
+			{
+				// Make any change in client b's session
+				Query:    "/* client b */ create table foo (i int);",
+				Expected: []sql.Row{{types.OkResult{RowsAffected: 0}}},
+			},
+			{
+				// Client a still has an unresolved FK at this point
+				Query:    "/* client a */ COMMIT;",
+				Expected: []sql.Row{},
+			},
+			{
+				// Assert that client a can see the schema with the foreign key constraints still present
+				Query:    "/* client a */ show create table t;",
+				Expected: []sql.Row{{"t", "CREATE TABLE `t` (\n  `id` int,\n  `ref_id` varchar(100),\n  KEY `ref_id` (`ref_id`),\n  CONSTRAINT `t_ibfk_1` FOREIGN KEY (`ref_id`) REFERENCES `ref` (`id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}},
+			},
+			{
+				// Assert that client b can see the schema with the foreign key constraints still present
+				Query:    "/* client b */ show create table t;",
+				Expected: []sql.Row{{"t", "CREATE TABLE `t` (\n  `id` int,\n  `ref_id` varchar(100),\n  KEY `ref_id` (`ref_id`),\n  CONSTRAINT `t_ibfk_1` FOREIGN KEY (`ref_id`) REFERENCES `ref` (`id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}},
+			},
+		},
+	},
 }
 
 var DoltConflictHandlingTests = []queries.TransactionTest{