Merge #54407

54407: sql: disallow cross-database foreign keys r=RaduBerinde a=RaduBerinde

This change disallows cross-database foreign key references and adds
a cluster setting to allow them (false by default).

Informs #54126.

Release note (sql change): creating cross-database foreign key
references is now disallowed (and can be re-enabled via a cluster
setting).

Co-authored-by: Radu Berinde <[email protected]>

docs/generated/settings/settings.html

@@ -46,6 +46,7 @@
 <tr><td><code>server.time_until_store_dead</code></td><td>duration</td><td><code>5m0s</code></td><td>the time after which if there is no new gossiped information about a store, it is considered dead</td></tr>
 <tr><td><code>server.user_login.timeout</code></td><td>duration</td><td><code>10s</code></td><td>timeout after which client authentication times out if some system range is unavailable (0 = no timeout)</td></tr>
 <tr><td><code>server.web_session_timeout</code></td><td>duration</td><td><code>168h0m0s</code></td><td>the duration that a newly created web session will be valid</td></tr>
+<tr><td><code>sql.cross_db_fks.enabled</code></td><td>boolean</td><td><code>false</code></td><td>if true, creating foreign key references across databases is allowed</td></tr>
 <tr><td><code>sql.defaults.default_int_size</code></td><td>integer</td><td><code>8</code></td><td>the size, in bytes, of an INT type</td></tr>
 <tr><td><code>sql.defaults.disallow_full_table_scans.enabled</code></td><td>boolean</td><td><code>false</code></td><td>setting to true rejects queries that have planned a full table scan</td></tr>
 <tr><td><code>sql.defaults.results_buffer.size</code></td><td>byte size</td><td><code>16 KiB</code></td><td>default size of the buffer that accumulates results for a statement or a batch of statements before they are sent to the client. This can be overridden on an individual connection with the 'results_buffer_size' parameter. Note that auto-retries generally only happen while no results have been delivered to the client, so reducing this size can increase the number of retriable errors a client receives. On the other hand, increasing the buffer size can increase the delay until the client receives the first result row. Updating the setting only affects new connections. Setting to 0 disables any buffering.</td></tr>

pkg/ccl/backupccl/show_test.go

@@ -39,6 +39,7 @@ func TestShowBackup(t *testing.T) {
 	defer cleanupFn()
 	defer cleanupEmptyCluster()
 	sqlDB.Exec(t, `
+SET CLUSTER SETTING sql.cross_db_fks.enabled = TRUE;
 CREATE TYPE data.welcome AS ENUM ('hello', 'hi');
 USE data; CREATE SCHEMA sc;
 CREATE TABLE data.sc.t1 (a INT);

pkg/cli/dump_test.go

@@ -708,6 +708,7 @@ INSERT INTO public.t2 (id, pkey) VALUES
 			name:     "dump_cross_references",
 			recreate: true,
 			create: `
+SET CLUSTER SETTING sql.cross_db_fks.enabled = TRUE;
 CREATE DATABASE dbB;
 USE dbB;
 

pkg/sql/alter_table.go

@@ -265,12 +265,11 @@ func (n *alterTableNode) startExec(params runParams) error {
 				// If there are any FKs, we will need to update the table descriptor of the
 				// depended-on table (to register this table against its DependedOnBy field).
 				// This descriptor must be looked up uncached, and we'll allow FK dependencies
-				// on tables that were just added. See the comment at the start of
-				// the global-scope resolveFK().
+				// on tables that were just added. See the comment at the start of ResolveFK().
 				// TODO(vivek): check if the cache can be used.
 				var err error
 				params.p.runWithOptions(resolveFlags{skipCache: true}, func() {
-					// Check whether the table is empty, and pass the result to resolveFK(). If
+					// Check whether the table is empty, and pass the result to ResolveFK(). If
 					// the table is empty, then resolveFK will automatically add the necessary
 					// index for a fk constraint if the index does not exist.
 					span := n.tableDesc.PrimaryIndexSpan(params.ExecCfg().Codec)
@@ -285,7 +284,17 @@ func (n *alterTableNode) startExec(params runParams) error {
 					} else {
 						tableState = NonEmptyTable
 					}
-					err = params.p.resolveFK(params.ctx, n.tableDesc, d, affected, tableState, t.ValidationBehavior)
+					err = ResolveFK(
+						params.ctx,
+						params.p.txn,
+						params.p,
+						n.tableDesc,
+						d,
+						affected,
+						tableState,
+						t.ValidationBehavior,
+						params.p.EvalContext(),
+					)
 				})
 				if err != nil {
 					return err

pkg/sql/create_table.go

@@ -513,21 +513,6 @@ func (n *createTableNode) Close(ctx context.Context) {
 	}
 }
 
-// resolveFK on the planner calls resolveFK() on the current txn.
-//
-// The caller must make sure the planner is configured to look up
-// descriptors without caching. See the comment on resolveFK().
-func (p *planner) resolveFK(
-	ctx context.Context,
-	tbl *tabledesc.Mutable,
-	d *tree.ForeignKeyConstraintTableDef,
-	backrefs map[descpb.ID]*tabledesc.Mutable,
-	ts FKTableState,
-	validationBehavior tree.ValidationBehavior,
-) error {
-	return ResolveFK(ctx, p.txn, p, tbl, d, backrefs, ts, validationBehavior, p.EvalContext())
-}
-
 func qualifyFKColErrorWithDB(
 	ctx context.Context, txn *kv.Txn, codec keys.SQLCodec, tbl *tabledesc.Mutable, col string,
 ) string {
@@ -547,7 +532,7 @@ func qualifyFKColErrorWithDB(
 	return tree.ErrString(tree.NewUnresolvedName(db.GetName(), schema, tbl.Name, col))
 }
 
-// FKTableState is the state of the referencing table resolveFK() is called on.
+// FKTableState is the state of the referencing table ResolveFK() is called on.
 type FKTableState int
 
 const (
@@ -662,6 +647,14 @@ func ResolveFK(
 	if err != nil {
 		return err
 	}
+	if target.ParentID != tbl.ParentID {
+		if !allowCrossDatabaseFKs.Get(&evalCtx.Settings.SV) {
+			return pgerror.Newf(pgcode.InvalidForeignKey,
+				"foreign references between databases are not allowed (see the '%s' cluster setting)",
+				allowCrossDatabaseFKsSetting,
+			)
+		}
+	}
 	if tbl.Temporary != target.Temporary {
 		persistenceType := "permanent"
 		if tbl.Temporary {
@@ -1205,7 +1198,7 @@ func newTableDescIfAs(
 // The caller must also ensure that the SchemaResolver is configured
 // to bypass caching and enable visibility of just-added descriptors.
 // This is used to resolve sequence and FK dependencies. Also see the
-// comment at the start of the global scope resolveFK().
+// comment at the start of ResolveFK().
 //
 // If the table definition *may* use the SERIAL type, the caller is
 // also responsible for processing serial types using
@@ -1820,7 +1813,7 @@ func newTableDesc(
 	// We need to run NewTableDesc with caching disabled, because
 	// it needs to pull in descriptors from FK depended-on tables
 	// and interleaved parents using their current state in KV.
-	// See the comment at the start of NewTableDesc() and resolveFK().
+	// See the comment at the start of NewTableDesc() and ResolveFK().
 	params.p.runWithOptions(resolveFlags{skipCache: true, contextDatabaseID: parentID}, func() {
 		ret, err = NewTableDesc(
 			params.ctx,

pkg/sql/exec_util.go

@@ -121,6 +121,14 @@ var defaultIntSize = func() *settings.IntSetting {
 	return s
 }()
 
+const allowCrossDatabaseFKsSetting = "sql.cross_db_fks.enabled"
+
+var allowCrossDatabaseFKs = settings.RegisterPublicBoolSetting(
+	allowCrossDatabaseFKsSetting,
+	"if true, creating foreign key references across databases is allowed",
+	false,
+)
+
 // traceTxnThreshold can be used to log SQL transactions that take
 // longer than duration to complete. For example, traceTxnThreshold=1s
 // will log the trace for any transaction that takes 1s or longer. To

pkg/sql/logictest/testdata/logic_test/fk

@@ -1,5 +1,8 @@
 # LogicTest: !3node-tenant(49854)
 
+statement ok
+SET CLUSTER SETTING sql.cross_db_fks.enabled = TRUE
+
 # Randomize the use of insert fast path.
 # The let statement will also log the value.
 let $enable_insert_fast_path
@@ -3551,3 +3554,75 @@ foreign key violation: "add_self_fk_fail" row b=3, k=1 has no match in "add_self
 
 statement ok
 DROP TABLE add_self_fk_fail
+
+# Test that we disallow cross-database foreign key references, depending on the
+# cluster setting.
+
+subtest cross_db_fks
+
+statement ok
+SET CLUSTER SETTING sql.cross_db_fks.enabled = FALSE
+
+statement ok
+CREATE DATABASE db1
+
+statement ok
+CREATE DATABASE db2
+
+statement ok
+USE db1
+
+statement ok
+CREATE TABLE parent (p INT PRIMARY KEY);
+
+statement ok
+CREATE TABLE child1 (c INT PRIMARY KEY, p INT REFERENCES parent(p))
+
+statement ok
+CREATE TABLE child2 (c INT PRIMARY KEY, p INT REFERENCES db1.public.parent(p))
+
+statement ok
+CREATE TABLE db1.public.child3 (c INT PRIMARY KEY, p INT REFERENCES db1.public.parent(p))
+
+statement error foreign references between databases are not allowed
+CREATE TABLE db2.public.child (c INT PRIMARY KEY, p INT REFERENCES parent(p))
+
+statement ok
+CREATE SCHEMA sc1
+
+# Cross-schema references should always be allowed.
+statement ok
+CREATE TABLE db1.sc1.child (c INT PRIMARY KEY, p INT REFERENCES db1.public.parent(p))
+
+statement ok
+USE db2
+
+statement error foreign references between databases are not allowed
+CREATE TABLE child (c INT PRIMARY KEY, p INT REFERENCES db1.public.parent(p))
+
+statement error foreign references between databases are not allowed
+CREATE TABLE db2.public.child (c INT PRIMARY KEY, p INT REFERENCES db1.public.parent(p))
+
+statement ok
+CREATE TABLE child (c INT PRIMARY KEY, p INT)
+
+statement error foreign references between databases are not allowed
+ALTER TABLE child ADD CONSTRAINT fk FOREIGN KEY (p) REFERENCES db1.public.parent(p)
+
+statement ok
+USE db1
+
+statement error foreign references between databases are not allowed
+ALTER TABLE db2.public.child ADD CONSTRAINT fk FOREIGN KEY (p) REFERENCES parent(p)
+
+statement ok
+SET CLUSTER SETTING sql.cross_db_fks.enabled = TRUE
+
+statement ok
+ALTER TABLE db2.public.child ADD CONSTRAINT fk FOREIGN KEY (p) REFERENCES parent(p)
+
+statement ok
+USE db2
+
+statement ok
+CREATE TABLE child2 (c INT PRIMARY KEY, p INT REFERENCES db1.public.parent(p))

pkg/sql/logictest/testdata/logic_test/reparent_database

@@ -1,4 +1,5 @@
 statement ok
+SET CLUSTER SETTING sql.cross_db_fks.enabled = TRUE;
 CREATE DATABASE pgdatabase;
 CREATE TABLE pgdatabase.t1 (x INT PRIMARY KEY);
 CREATE DATABASE pgschema;

pkg/sql/show_test.go

@@ -42,6 +42,7 @@ func TestShowCreateTable(t *testing.T) {
 	defer s.Stopper().Stop(context.Background())
 
 	if _, err := sqlDB.Exec(`
+    SET CLUSTER SETTING sql.cross_db_fks.enabled = TRUE;
 		CREATE DATABASE d;
 		SET DATABASE = d;
 		CREATE TABLE items (