Merge #34365

34365: sql: fix FK validation join implementation r=lucy-zhang a=lucy-zhang This PR updates the SQL query used for VALIDATE CONSTRAINT for foreign keys. The new implementation is compatible with the recent changes to FK matching semantics (both MATCH FULL and MATCH SIMPLE). It also uses a merge join, which will improve performance significantly compared to the old hash join implementation. Release note (sql change): VALIDATE CONSTRAINT for foreign keys is now compatible with the new MATCH FULL and MATCH SIMPLE semantics, and is more performant. Fixes #33452 Co-authored-by: Lucy Zhang <[email protected]>
cockroachdb · Feb 5, 2019 · 9db4764 · 9db4764
2 parents 8202611 + e53dab6
commit 9db4764
Show file tree

Hide file tree

Showing 2 changed files with 651 additions and 28 deletions.
diff --git a/pkg/sql/check.go b/pkg/sql/check.go
@@ -61,6 +61,81 @@ func validateCheckExpr(
 	return nil
 }
 
+// matchFullUnacceptableKeyQuery generates and returns a query for rows that are
+// disallowed given the specified MATCH FULL composite FK reference, i.e., rows
+// in the referencing table where the key contains both null and non-null
+// values.
+//
+// For example, a FK constraint on columns (a_id, b_id) with an index c_id on
+// the table "child" would require the following query:
+//
+// SELECT * FROM child@c_idx
+// WHERE
+//   NOT ((COALESCE(a_id, b_id) IS NULL) OR (a_id IS NOT NULL AND b_id IS NOT NULL))
+// LIMIT 1;
+func matchFullUnacceptableKeyQuery(
+	prefix int, srcName *string, srcIdx *sqlbase.IndexDescriptor,
+) string {
+	srcCols, srcNotNullClause := make([]string, prefix), make([]string, prefix)
+	for i := 0; i < prefix; i++ {
+		srcCols[i] = tree.NameString(srcIdx.ColumnNames[i])
+		srcNotNullClause[i] = fmt.Sprintf("%s IS NOT NULL", tree.NameString(srcIdx.ColumnNames[i]))
+	}
+	return fmt.Sprintf(
+		`SELECT * FROM %s@%s WHERE NOT ((COALESCE(%s) IS NULL) OR (%s)) LIMIT 1`,
+		*srcName, tree.NameString(srcIdx.Name),
+		strings.Join(srcCols, ", "),
+		strings.Join(srcNotNullClause, " AND "),
+	)
+}
+
+// nonMatchingRowQuery generates and returns a query for rows that violate the
+// specified FK constraint, i.e., rows in the referencing table with no matching
+// key in the referenced table. Rows in the referencing table with any null
+// values in the key are excluded from matching (for both MATCH FULL and MATCH
+// SIMPLE).
+//
+// For example, a FK constraint on columns (a_id, b_id) with an index c_id on
+// the table "child", referencing columns (a, b) with an index p_id on the table
+// "parent", would require the following query:
+//
+// SELECT
+//   s.a_id, s.b_id
+// FROM
+//   (SELECT * FROM child@c_idx WHERE a_id IS NOT NULL AND b_id IS NOT NULL) AS s
+//   LEFT OUTER JOIN parent@p_idx AS t ON s.a_id = t.a AND s.b_id = t.b
+// WHERE
+//   t.a IS NULL
+// LIMIT 1;
+func nonMatchingRowQuery(
+	prefix int,
+	srcName *string,
+	srcIdx *sqlbase.IndexDescriptor,
+	targetName *string,
+	targetIdx *sqlbase.IndexDescriptor,
+) string {
+	srcCols, srcWhere, targetCols, on := make([]string, prefix), make([]string, prefix), make([]string, prefix), make([]string, prefix)
+
+	for i := 0; i < prefix; i++ {
+		// s and t are table aliases used in the query
+		srcCols[i] = fmt.Sprintf("s.%s", tree.NameString(srcIdx.ColumnNames[i]))
+		srcWhere[i] = fmt.Sprintf("%s IS NOT NULL", tree.NameString(srcIdx.ColumnNames[i]))
+		targetCols[i] = fmt.Sprintf("t.%s", tree.NameString(targetIdx.ColumnNames[i]))
+		on[i] = fmt.Sprintf("%s = %s", srcCols[i], targetCols[i])
+	}
+
+	return fmt.Sprintf(
+		`SELECT %s FROM (SELECT * FROM %s@%s WHERE %s) AS s LEFT OUTER JOIN %s@%s AS t ON %s WHERE %s IS NULL LIMIT 1`,
+		strings.Join(srcCols, ", "),
+		*srcName, tree.NameString(srcIdx.Name),
+		strings.Join(srcWhere, " AND "),
+		*targetName, tree.NameString(targetIdx.Name),
+		strings.Join(on, " AND "),
+		// Sufficient to check the first column to see whether there was no matching row
+		targetCols[0],
+	)
+}
+
 func (p *planner) validateForeignKey(
 	ctx context.Context, srcTable *sqlbase.TableDescriptor, srcIdx *sqlbase.IndexDescriptor,
 ) error {
@@ -88,28 +163,54 @@ func (p *planner) validateForeignKey(
 		prefix = p
 	}
 
-	srcCols, targetCols := make([]string, prefix), make([]string, prefix)
-	join, where := make([]string, prefix), make([]string, prefix)
+	// For MATCH FULL FKs, first check whether any disallowed keys containing both
+	// null and non-null values exist.
+	// (The matching options only matter for FKs with more than one column.)
+	if prefix > 1 && srcIdx.ForeignKey.Match == sqlbase.ForeignKeyReference_FULL {
+		query := matchFullUnacceptableKeyQuery(prefix, &srcName, srcIdx)
 
-	for i := 0; i < prefix; i++ {
-		srcCols[i] = fmt.Sprintf("s.%s", tree.NameString(srcIdx.ColumnNames[i]))
-		targetCols[i] = fmt.Sprintf("t.%s", tree.NameString(targetIdx.ColumnNames[i]))
-		join[i] = fmt.Sprintf("(%s = %s OR (%s IS NULL AND %s IS NULL))",
-			srcCols[i], targetCols[i], srcCols[i], targetCols[i])
-		where[i] = fmt.Sprintf("(%s IS NOT NULL AND %s IS NULL)", srcCols[i], targetCols[i])
-	}
+		log.Infof(ctx, "Validating MATCH FULL FK %q (%q [%v] -> %q [%v]) with query %q",
+			srcIdx.ForeignKey.Name,
+			srcTable.Name, srcIdx.ColumnNames, targetTable.Name, targetIdx.ColumnNames,
+			query,
+		)
 
-	query := fmt.Sprintf(
-		`SELECT %s FROM %s@%s AS s LEFT OUTER JOIN %s@%s AS t ON %s WHERE %s LIMIT 1`,
-		strings.Join(srcCols, ", "),
-		srcName, tree.NameString(srcIdx.Name), targetName, tree.NameString(targetIdx.Name),
-		strings.Join(join, " AND "),
-		strings.Join(where, " OR "),
-	)
+		rows, err := p.delegateQuery(ctx, "ALTER TABLE VALIDATE", query, nil, nil)
+		if err != nil {
+			return err
+		}
+
+		rows, err = p.optimizePlan(ctx, rows, allColumns(rows))
+		if err != nil {
+			return err
+		}
+		defer rows.Close(ctx)
+
+		params := runParams{
+			ctx:             ctx,
+			extendedEvalCtx: &p.extendedEvalCtx,
+			p:               p,
+		}
+		if err := startPlan(params, rows); err != nil {
+			return err
+		}
+		next, err := rows.Next(params)
+		if err != nil {
+			return err
+		}
+
+		if next {
+			return pgerror.NewErrorf(pgerror.CodeForeignKeyViolationError,
+				"foreign key violation: MATCH FULL does not allow mixing of null and nonnull values %s for %s",
+				rows.Values(), srcIdx.ForeignKey.Name,
+			)
+		}
+	}
+	query := nonMatchingRowQuery(prefix, &srcName, srcIdx, &targetName, targetIdx)
 
 	log.Infof(ctx, "Validating FK %q (%q [%v] -> %q [%v]) with query %q",
 		srcIdx.ForeignKey.Name,
-		srcTable.Name, srcCols, targetTable.Name, targetCols,
+		srcTable.Name, srcIdx.ColumnNames, targetTable.Name, targetIdx.ColumnNames,
 		query,
 	)