sql: disallow cross-database sequence references

Fixes: #69992

Previously, we incorrectly allowed cross DB references
for sequences. This was inadequate because we are going
to drop support for cross DB references. To address this
this patch will disable support for cross DB sequence
references by default.

Release note (sql change): Disallow cross DB references for sequences by
default. This can be enabled with the cluster setting
sql.cross_db_sequence_references.enabled.

docs/generated/settings/settings-for-tenants.txt

@@ -74,6 +74,7 @@ server.web_session.purge.ttl	duration	1h0m0s	if nonzero, entries in system.web_s
 server.web_session_timeout	duration	168h0m0s	the duration that a newly created web session will be valid
 sql.cross_db_fks.enabled	boolean	false	if true, creating foreign key references across databases is allowed
 sql.cross_db_sequence_owners.enabled	boolean	false	if true, creating sequences owned by tables from other databases is allowed
+sql.cross_db_sequence_references.enabled	boolean	false	if true, sequences referenced by tables from other databases are allowed
 sql.cross_db_views.enabled	boolean	false	if true, creating views that refer to other databases is allowed
 sql.defaults.copy_partitioning_when_deinterleaving_table.enabled	boolean	false	default value for enable_copying_partitioning_when_deinterleaving_table session variable
 sql.defaults.datestyle	enumeration	iso, mdy	default value for DateStyle session setting [iso, mdy = 0, iso, dmy = 1, iso, ymd = 2]

docs/generated/settings/settings.html

@@ -79,6 +79,7 @@
 <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.cross_db_sequence_owners.enabled</code></td><td>boolean</td><td><code>false</code></td><td>if true, creating sequences owned by tables from other databases is allowed</td></tr>
+<tr><td><code>sql.cross_db_sequence_references.enabled</code></td><td>boolean</td><td><code>false</code></td><td>if true, sequences referenced by tables from other databases are allowed</td></tr>
 <tr><td><code>sql.cross_db_views.enabled</code></td><td>boolean</td><td><code>false</code></td><td>if true, creating views that refer to other databases is allowed</td></tr>
 <tr><td><code>sql.defaults.copy_partitioning_when_deinterleaving_table.enabled</code></td><td>boolean</td><td><code>false</code></td><td>default value for enable_copying_partitioning_when_deinterleaving_table session variable</td></tr>
 <tr><td><code>sql.defaults.datestyle</code></td><td>enumeration</td><td><code>iso, mdy</code></td><td>default value for DateStyle session setting [iso, mdy = 0, iso, dmy = 1, iso, ymd = 2]</td></tr>

pkg/sql/exec_util.go

@@ -168,6 +168,14 @@ var allowCrossDatabaseSeqOwner = settings.RegisterBoolSetting(
 	false,
 ).WithPublic()
 
+const allowCrossDatabaseSeqReferencesSetting = "sql.cross_db_sequence_references.enabled"
+
+var allowCrossDatabaseSeqReferences = settings.RegisterBoolSetting(
+	allowCrossDatabaseSeqReferencesSetting,
+	"if true, sequences referenced by tables from other databases are allowed",
+	false,
+).WithPublic()
+
 const secondaryTenantsZoneConfigsEnabledSettingName = "sql.zone_configs.experimental_allow_for_secondary_tenant.enabled"
 
 // secondaryTenantZoneConfigsEnabled controls if secondary tenants are allowed

pkg/sql/logictest/testdata/logic_test/drop_database

@@ -1,6 +1,9 @@
 statement ok
 SET CLUSTER SETTING sql.cross_db_views.enabled = TRUE
 
+statement ok
+SET CLUSTER SETTING sql.cross_db_sequence_references.enabled = TRUE
+
 statement ok
 CREATE DATABASE "foo-bar"
 

pkg/sql/logictest/testdata/logic_test/drop_sequence

@@ -3,6 +3,9 @@
 statement ok
 SET sql_safe_updates = true
 
+statement ok
+SET CLUSTER SETTING sql.cross_db_sequence_references.enabled = TRUE
+
 # Test dropping sequences with/without CASCADE
 subtest drop_sequence
 

pkg/sql/logictest/testdata/logic_test/rename_database

@@ -3,6 +3,9 @@
 statement ok
 SET CLUSTER SETTING sql.cross_db_views.enabled = TRUE
 
+statement ok
+SET CLUSTER SETTING sql.cross_db_sequence_references.enabled = TRUE
+
 query TTTTT
 SHOW DATABASES
 ----

pkg/sql/logictest/testdata/logic_test/sequences

@@ -6,6 +6,10 @@
 statement ok
 SET CLUSTER SETTING sql.cross_db_sequence_owners.enabled = TRUE
 
+statement ok
+SET CLUSTER SETTING sql.cross_db_sequence_references.enabled = TRUE
+
+
 statement ok
 SET DATABASE = test
 
@@ -1816,3 +1820,16 @@ SELECT * FROM "".crdb_internal.cross_db_references;
 db2  public  seq   db1  public  t  sequences owning table
 db2  public  seq2  db1  public  t  sequences owning table
 test  public  tdb3ref  db3  public  s  table column refers to sequence
+
+statement ok
+SET CLUSTER SETTING sql.cross_db_sequence_references.enabled = FALSE
+
+# Validate that cross DB sequences are detected by internal tables
+statement ok
+CREATE DATABASE db4;
+
+statement ok
+CREATE SEQUENCE db4.s;
+
+statement error pq: sequence references cannot come from other databases; \(see the 'sql\.cross_db_sequence_references\.enabled' cluster setting\)
+CREATE TABLE tDb4Ref (i INT PRIMARY KEY DEFAULT (nextval('db4.s')));

pkg/sql/logictest/testdata/logic_test/sequences_regclass

@@ -28,6 +28,12 @@ ALTER TABLE foo ADD COLUMN k SERIAL
 statement ok
 ALTER TABLE foo ADD COLUMN l INT NOT NULL
 
+statement error pq: sequence references cannot come from other databases; \(see the 'sql\.cross_db_sequence_references\.enabled' cluster setting\)
+ALTER TABLE FOO ALTER COLUMN l SET DEFAULT currval('diff_db.test_seq')
+
+statement ok
+SET CLUSTER SETTING sql.cross_db_sequence_references.enabled = TRUE
+
 statement ok
 ALTER TABLE FOO ALTER COLUMN l SET DEFAULT currval('diff_db.test_seq')
 

pkg/sql/rename_table.go

@@ -334,7 +334,7 @@ func (n *renameTableNode) checkForCrossDbReferences(
 	// Validates if a given dependency on a relation will
 	// lead to a cross DB reference, and an appropriate
 	// error is generated.
-	checkDepForCrossDbRef := func(depID descpb.ID) error {
+	checkDepForCrossDbRef := func(depID descpb.ID, isOwner bool) error {
 		dependentObject, err := p.Descriptors().GetImmutableTableByID(ctx, p.txn, depID,
 			tree.ObjectLookupFlags{
 				CommonLookupFlags: tree.CommonLookupFlags{
@@ -360,7 +360,8 @@ func (n *renameTableNode) checkForCrossDbReferences(
 						allowCrossDatabaseViewsSetting),
 					crossDBReferenceDeprecationHint(),
 				)
-			} else if !allowCrossDatabaseSeqOwner.Get(&p.execCfg.Settings.SV) &&
+			} else if isOwner &&
+				!allowCrossDatabaseSeqOwner.Get(&p.execCfg.Settings.SV) &&
 				dependentObject.IsSequence() {
 				return errors.WithHintf(
 					pgerror.Newf(pgcode.ObjectNotInPrerequisiteState,
@@ -371,6 +372,17 @@ func (n *renameTableNode) checkForCrossDbReferences(
 					crossDBReferenceDeprecationHint(),
 				)
 			}
+		} else if !isOwner &&
+			!allowCrossDatabaseSeqReferences.Get(&p.execCfg.Settings.SV) &&
+			dependentObject.IsSequence() {
+			return errors.WithHintf(
+				pgerror.Newf(pgcode.ObjectNotInPrerequisiteState,
+					"a sequence %q will be referenced BY a table in a different database after rename "+
+						"(see the '%s' cluster setting)",
+					dependentObject.GetName(),
+					allowCrossDatabaseSeqReferencesSetting),
+				crossDBReferenceDeprecationHint(),
+			)
 		} else if tableDesc.IsView() {
 			// For views it can only be a relation.
 			return errors.WithHintf(
@@ -381,7 +393,8 @@ func (n *renameTableNode) checkForCrossDbReferences(
 					allowCrossDatabaseViewsSetting),
 				crossDBReferenceDeprecationHint(),
 			)
-		} else if tableDesc.IsSequence() {
+		} else if isOwner &&
+			tableDesc.IsSequence() {
 			return errors.WithHintf(
 				pgerror.Newf(pgcode.ObjectNotInPrerequisiteState,
 					"this sequence will be OWNED BY a table %q in a different database after rename "+
@@ -390,6 +403,16 @@ func (n *renameTableNode) checkForCrossDbReferences(
 					allowCrossDatabaseSeqOwnerSetting),
 				crossDBReferenceDeprecationHint(),
 			)
+		} else if !isOwner &&
+			tableDesc.IsSequence() {
+			return errors.WithHintf(
+				pgerror.Newf(pgcode.ObjectNotInPrerequisiteState,
+					"this sequence will be referenced BY a table %q in a different database after rename "+
+						"(see the '%s' cluster setting)",
+					dependentObject.GetName(),
+					allowCrossDatabaseSeqReferencesSetting),
+				crossDBReferenceDeprecationHint(),
+			)
 		}
 		return nil
 	}
@@ -439,23 +462,32 @@ func (n *renameTableNode) checkForCrossDbReferences(
 
 		// If cross database sequence owners are not allowed, then
 		// check if any column owns a sequence.
-		if !allowCrossDatabaseSeqOwner.Get(&p.execCfg.Settings.SV) {
-			for _, columnDesc := range tableDesc.Columns {
+		for _, columnDesc := range tableDesc.Columns {
+			if !allowCrossDatabaseSeqOwner.Get(&p.execCfg.Settings.SV) {
 				for _, ownsSequenceID := range columnDesc.OwnsSequenceIds {
-					err := checkDepForCrossDbRef(ownsSequenceID)
+					err := checkDepForCrossDbRef(ownsSequenceID, true)
 					if err != nil {
 						return err
 					}
 				}
 			}
+			if !allowCrossDatabaseSeqReferences.Get(&p.execCfg.Settings.SV) {
+				for _, seqID := range columnDesc.UsesSequenceIds {
+					err := checkDepForCrossDbRef(seqID, false)
+					if err != nil {
+						return err
+					}
+				}
+
+			}
 		}
 
 		// Check if any views depend on this table, while
 		// DependsOnBy contains sequences these are only
 		// once that are in use.
 		if !allowCrossDatabaseViews.Get(&p.execCfg.Settings.SV) {
 			err := tableDesc.ForeachDependedOnBy(func(dep *descpb.TableDescriptor_Reference) error {
-				return checkDepForCrossDbRef(dep.ID)
+				return checkDepForCrossDbRef(dep.ID, false)
 			})
 			if err != nil {
 				return err
@@ -466,7 +498,7 @@ func (n *renameTableNode) checkForCrossDbReferences(
 		// For views check if we depend on tables in a different database.
 		dependsOn := tableDesc.GetDependsOn()
 		for _, dependency := range dependsOn {
-			err := checkDepForCrossDbRef(dependency)
+			err := checkDepForCrossDbRef(dependency, false)
 			if err != nil {
 				return err
 			}
@@ -479,15 +511,26 @@ func (n *renameTableNode) checkForCrossDbReferences(
 				return err
 			}
 		}
-	} else if tableDesc.IsSequence() &&
-		!allowCrossDatabaseSeqOwner.Get(&p.execCfg.Settings.SV) {
-		// For sequences check if the sequence is owned by
-		// a different database.
-		sequenceOpts := tableDesc.GetSequenceOpts()
-		if sequenceOpts.SequenceOwner.OwnerTableID != descpb.InvalidID {
-			err := checkDepForCrossDbRef(sequenceOpts.SequenceOwner.OwnerTableID)
-			if err != nil {
-				return err
+	} else if tableDesc.IsSequence() {
+		// Check if the sequence is owned by a different database.
+		if !allowCrossDatabaseSeqOwner.Get(&p.execCfg.Settings.SV) {
+			sequenceOpts := tableDesc.GetSequenceOpts()
+			if sequenceOpts.SequenceOwner.OwnerTableID != descpb.InvalidID {
+				err := checkDepForCrossDbRef(sequenceOpts.SequenceOwner.OwnerTableID, true)
+				if err != nil {
+					return err
+				}
+			}
+		}
+		// Check if a table in a different database depends on this
+		// sequence.
+		if !allowCrossDatabaseSeqReferences.Get(&p.execCfg.Settings.SV) {
+
+			for _, sequenceReferences := range tableDesc.GetDependedOnBy() {
+				err := checkDepForCrossDbRef(sequenceReferences.ID, false)
+				if err != nil {
+					return err
+				}
 			}
 		}
 	}

pkg/sql/sequence.go

@@ -653,6 +653,17 @@ func maybeAddSequenceDependencies(
 		if err != nil {
 			return nil, err
 		}
+		// Check if this reference is cross DB.
+		if seqDesc.GetParentID() != tableDesc.GetParentID() &&
+			!allowCrossDatabaseSeqReferences.Get(&st.SV) {
+			return nil, errors.WithHintf(
+				pgerror.Newf(pgcode.FeatureNotSupported,
+					"sequence references cannot come from other databases; (see the '%s' cluster setting)",
+					allowCrossDatabaseSeqReferencesSetting),
+				crossDBReferenceDeprecationHint(),
+			)
+
+		}
 		seqNameToID[seqIdentifier.SeqName] = int64(seqDesc.ID)
 
 		// If we had already modified this Sequence as part of this transaction,