Skip to content

sql: foreign key relationships should encode columns, not indexes #37255

@jordanlewis

Description

@jordanlewis

Currently, foreign key relationships are encoded on IndexDescriptors via the ForeignKey and ReferencedBy fields. This encoding has several unfortunate side-effects:

  1. foreign keys are "tied" to indexes: instead of directly stating the columns of the referenced or referencing tables that make up the relationship, they specify a particular index. This currently limits checks to occur on that index, even though there might be a better one to look at depending on locality or other optimizer-driven choices, once the optimizer is involved in planning FK checks.
  2. a table that references another table must have an index on the columns that are referencing, to permit checked deletes from the referenced table. For example, a customers -> zipcodes foreign key must have an index on the customers.zipcode column, even though the zipcodes table might never be deleted from in reality - this is unfortunate because each index has an overhead on insert.
  3. an index can't have more than one outgoing foreign key

I'm starting this issue to centralize any ongoing discussions. I'm assigning it to the Schema changes project, but it's possible that another team will ultimately do the work.

To avoid this situation, we should move the knowledge about foreign keys out of IndexDescriptor and into TableDescriptor. Further, the ForeignKeyReference protobuf should be changed to encode columns instead of indexes. Here's a proposed diff:

diff --git a/pkg/sql/sqlbase/structured.proto b/pkg/sql/sqlbase/structured.proto
index 28e917fbc7..6e53b71584 100644
--- a/pkg/sql/sqlbase/structured.proto
+++ b/pkg/sql/sqlbase/structured.proto
@@ -50,18 +50,25 @@ message ForeignKeyReference {
     PARTIAL = 2; // Note: not actually supported, but we reserve the value for future use.
   }

-  optional uint32 table = 1 [(gogoproto.nullable) = false, (gogoproto.casttype) = "ID"];
-  optional uint32 index = 2 [(gogoproto.nullable) = false, (gogoproto.casttype) = "IndexID"];
+  optional uint32 referenced_table = 1 [(gogoproto.nullable) = false, (gogoproto.casttype) = "ID"];
+  optional uint32 index = 2 [(gogoproto.nullable) = false, (gogoproto.casttype) = "IndexID", deprecated=true];
   optional string name = 3 [(gogoproto.nullable) = false];
   optional ConstraintValidity validity = 4 [(gogoproto.nullable) = false];
   // If this FK only uses a prefix of the columns in its index, we record how
   // many to avoid spuriously counting the additional cols as used by this FK.
-  optional int32 shared_prefix_len = 5 [(gogoproto.nullable) = false];
+  optional int32 shared_prefix_len = 5 [(gogoproto.nullable) = false, deprecated=true];
   optional Action on_delete = 6 [(gogoproto.nullable) = false];
   optional Action on_update = 7 [(gogoproto.nullable) = false];
   // This is only important for composite keys. For all prior matches before
   // the addition of this value, MATCH SIMPLE will be used.
   optional Match match = 8 [(gogoproto.nullable) = false];
+  // self_column_ids contains the column ids in this table that make up the
+  // reference.
+  repeated uint32 self_column_ids = 9;
+  // other_column_ids contains the column ids in referenced_table that
+  // correspond to the self_column_ids in this table. This field must be the
+  // same length as self_column_ids.
+  repeated uint32 other_column_ids = 10;
 }

 message ColumnDescriptor {
@@ -306,8 +313,8 @@ message IndexDescriptor {
   repeated uint32 composite_column_ids = 13
       [(gogoproto.customname) = "CompositeColumnIDs", (gogoproto.casttype) = "ColumnID"];

-  optional ForeignKeyReference foreign_key = 9 [(gogoproto.nullable) = false];
-  repeated ForeignKeyReference referenced_by = 10 [(gogoproto.nullable) = false];
+  optional ForeignKeyReference foreign_key = 9 [(gogoproto.nullable) = false, deprecated=true];
+  repeated ForeignKeyReference referenced_by = 10 [(gogoproto.nullable) = false, deprecated=true];

   // Interleave, if it's not the zero value, describes how this index's data is
   // interleaved into another index's data.
@@ -729,6 +736,23 @@ message TableDescriptor {
   // index case. Also use for dropped interleaved indexes and columns.
   repeated GCDescriptorMutation gc_mutations = 33 [(gogoproto.nullable) = false,
                                                   (gogoproto.customname) = "GCMutations"];
+
+  // out_fks contains the foreign key references from this table to other
+  // tables, which is needed so inserts into this table can trigger checks on
+  // the referenced tables to ensure that the new data has corresponding rows.
+  repeated ForeignKeyReference out_fks = 34;
+  // in_fks contains the foreign key references from other tables to this
+  // table, which is needed so deletions or updates to this table can trigger
+  // checks on the referencing tables to ensure that no foreign key references
+  // are broken by the changes. These are the "backward references".
+  //
+  // As an example of how out_fks and in_fks work, consider two tables:
+  // customers (id int, zipcode int) and zipcodes (zipcode int), with a foreign
+  // key from customers.zipcode to zipcodes.zipcode. The out_fks field on
+  // customers would contain a ForeignKeyReference with self_column_ids = 1 and
+  // other_column_ids = 0, and the in_fks field on zipcodes would contain a
+  // ForeignKeyReference with self_column_ids = 0 and other_column_ids = 1.
+  repeated ForeignKeyReference in_fks = 35;
 }

 // DatabaseDescriptor represents a namespace (aka database) and is stored

Note that the old fields are deprecated and not deleted, because the code for the release that includes this upgrades will have to migrate descriptors from the old format to the new format, while preserving the old format so that mixed-version clusters can still work properly. The release after can delete the code that understands the old format, as well as change the deprecated fields to reserved (empty) ones.

Metadata

Metadata

Assignees

Labels

A-sql-fksC-cleanupTech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions