Merge pull request #7042 from dolthub/nicktobey/schemamergetestsfix

Run nullness checking on merged rows during three-way merges.

go/libraries/doltcore/merge/merge_prolly_rows.go

@@ -888,6 +888,7 @@ func (nv nullValidator) validateDiff(ctx context.Context, diff tree.ThreeWayDiff
 			}
 		}
 		count = len(violations)
+		return
 
 	case tree.DiffOpLeftAdd, tree.DiffOpLeftModify:
 		var violations []string
@@ -930,6 +931,37 @@ func (nv nullValidator) validateDiff(ctx context.Context, diff tree.ThreeWayDiff
 			}
 		}
 		count = len(violations)
+		return
+	case tree.DiffOpDivergentModifyResolved:
+		var violations []string
+		for to, _ := range nv.leftMap {
+			col := nv.final.GetNonPKCols().GetByIndex(to)
+			if !col.IsNullable() && diff.Merged.FieldIsNull(to) {
+				violations = append(violations, col.Name)
+			}
+		}
+		// for merged NULL violations, we insert a constraint violation and
+		// then must explicitly remove this row from all left-side indexes
+		if len(violations) > 0 {
+			var meta prolly.ConstraintViolationMeta
+			if meta, err = newNotNullViolationMeta(violations, diff.Merged); err != nil {
+				return 0, err
+			}
+			err = nv.artEditor.ReplaceConstraintViolation(ctx, diff.Key, nv.ourRootish, prolly.ArtifactTypeNullViol, meta)
+			if err != nil {
+				return 0, err
+			}
+			if err = nv.leftEditor.Delete(ctx, diff.Key); err != nil {
+				return 0, err
+			}
+			for _, editor := range nv.secEditors {
+				if err = editor.DeleteEntry(ctx, diff.Key, diff.Left); err != nil {
+					return 0, err
+				}
+			}
+		}
+		count = len(violations)
+		return
 	}
 	return
 }
@@ -1922,5 +1954,9 @@ func convert(ctx context.Context, fromDesc, toDesc val.TupleDesc, toSchema schem
 	if err != nil {
 		return nil, err
 	}
-	return index.Serialize(ctx, ns, toDesc.Types[toIndex], convertedCell)
+	typ := toDesc.Types[toIndex]
+	// If a merge results in assigning NULL to a non-null column, don't panic.
+	// Instead we validate the merged tuple before merging it into the table.
+	typ.Nullable = true
+	return index.Serialize(ctx, ns, typ, convertedCell)
 }

go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go

@@ -1725,6 +1725,38 @@ var MergeScripts = []queries.ScriptTest{
 			},
 		},
 	},
+	{
+		Name: "resolving a modified/modified row still checks nullness constraint",
+		SetUpScript: []string{
+			"create table test(a int primary key, b int, c int);",
+			"insert into test values (1, 2, 3);",
+			"call dolt_add('test');",
+			"call dolt_commit('-m', 'create test table');",
+
+			"call dolt_checkout('-b', 'other');",
+			"alter table test modify c int not null;",
+			"update test set b = NULL;",
+			"call dolt_add('test');",
+			"call dolt_commit('-m', 'drop column');",
+
+			"call dolt_checkout('main');",
+			"alter table test modify b int not null",
+			"update test set c = NULL;",
+			"call dolt_add('test');",
+			"call dolt_commit('-m', 'remove row');",
+			"set dolt_force_transaction_commit = on;",
+		},
+		Assertions: []queries.ScriptTestAssertion{
+			{
+				Query:    "CALL DOLT_MERGE('other');",
+				Expected: []sql.Row{{"", 0, 1}},
+			},
+			{
+				Query:    "select a, b, c from dolt_constraint_violations_test;",
+				Expected: []sql.Row{{1, nil, nil}},
+			},
+		},
+	},
 	{
 		Name: "Pk convergent updates to sec diff congruent",
 		SetUpScript: []string{