sql: allow schema changes on tables in state: TableDescriptor_ADD

A new table descriptor is placed in the non-public TableDescriptor_ADD state
when it refers to another table through an FK or interleaved reference,
and can only be made public once the other tables back reference is
visible across the cluster.

This change allows schema change operations on such non-public tables,
while preserving the non-public behavior for table leases.

fixes #13505

pkg/sql/create.go

@@ -144,7 +144,7 @@ func (p *planner) CreateIndex(ctx context.Context, n *parser.CreateIndex) (planN
 		return nil, err
 	}
 
-	tableDesc, err := mustGetTableDesc(ctx, p.txn, p.getVirtualTabler(), tn)
+	tableDesc, err := mustGetTableDesc(ctx, p.txn, p.getVirtualTabler(), tn, true /*allowAdding*/)
 	if err != nil {
 		return nil, err
 	}
@@ -1053,7 +1053,7 @@ func addInterleave(
 		return err
 	}
 
-	parentTable, err := mustGetTableDesc(ctx, txn, vt, tn)
+	parentTable, err := mustGetTableDesc(ctx, txn, vt, tn, true /*allowAdding*/)
 	if err != nil {
 		return err
 	}

pkg/sql/data_source.go

@@ -492,16 +492,18 @@ func (p *planner) getTableScanOrViewPlan(
 	hints *parser.IndexHints,
 	scanVisibility scanVisibility,
 ) (planDataSource, error) {
-	descFunc := p.session.leases.getTableLease
+	var desc *sqlbase.TableDescriptor
+	var err error
 	if p.avoidCachedDescriptors {
 		// AS OF SYSTEM TIME queries need to fetch the table descriptor at the
 		// specified time, and never lease anything. The proto transaction already
 		// has its timestamps set correctly so getTableOrViewDesc will fetch with
 		// the correct timestamp.
-		descFunc = mustGetTableOrViewDesc
+		desc, err = mustGetTableOrViewDesc(
+			ctx, p.txn, p.getVirtualTabler(), tn, false /*allowAdding*/)
+	} else {
+		desc, err = p.session.leases.getTableLease(ctx, p.txn, p.getVirtualTabler(), tn)
 	}
-
-	desc, err := descFunc(ctx, p.txn, p.getVirtualTabler(), tn)
 	if err != nil {
 		return planDataSource{}, err
 	}

pkg/sql/descriptor.go

@@ -268,7 +268,8 @@ func getDescriptorsFromTargetList(
 			return nil, err
 		}
 		for i := range tables {
-			descriptor, err := mustGetTableOrViewDesc(ctx, txn, vt, &tables[i])
+			descriptor, err := mustGetTableOrViewDesc(
+				ctx, txn, vt, &tables[i], true /*allowAdding*/)
 			if err != nil {
 				return nil, err
 			}

pkg/sql/drop.go

@@ -243,7 +243,7 @@ func (p *planner) DropIndex(ctx context.Context, n *parser.DropIndex) (planNode,
 			return nil, err
 		}
 
-		tableDesc, err := mustGetTableDesc(ctx, p.txn, p.getVirtualTabler(), tn)
+		tableDesc, err := mustGetTableDesc(ctx, p.txn, p.getVirtualTabler(), tn, true /*allowAdding*/)
 		if err != nil {
 			return nil, err
 		}

pkg/sql/rename.go

@@ -258,7 +258,7 @@ func (p *planner) RenameIndex(ctx context.Context, n *parser.RenameIndex) (planN
 		return nil, err
 	}
 
-	tableDesc, err := mustGetTableDesc(ctx, p.txn, p.getVirtualTabler(), tn)
+	tableDesc, err := mustGetTableDesc(ctx, p.txn, p.getVirtualTabler(), tn, true /*allowAdding*/)
 	if err != nil {
 		return nil, err
 	}

pkg/sql/show.go

@@ -364,7 +364,7 @@ func (p *planner) ShowCreateTable(
 		return nil, err
 	}
 
-	desc, err := mustGetTableDesc(ctx, p.txn, p.getVirtualTabler(), tn)
+	desc, err := mustGetTableDesc(ctx, p.txn, p.getVirtualTabler(), tn, true /*allowAdding*/)
 	if err != nil {
 		return nil, err
 	}
@@ -819,7 +819,7 @@ func (p *planner) ShowConstraints(
 		return nil, err
 	}
 
-	desc, err := mustGetTableDesc(ctx, p.txn, p.getVirtualTabler(), tn)
+	desc, err := mustGetTableDesc(ctx, p.txn, p.getVirtualTabler(), tn, true /*allowAdding*/)
 	if err != nil {
 		return nil, err
 	}

pkg/sql/show_fingerprints.go

@@ -68,7 +68,8 @@ func (p *planner) ShowFingerprints(
 		txn.SetFixedTimestamp(ts)
 
 		var err error
-		tableDesc, err = mustGetTableDesc(ctx, txn, p.getVirtualTabler(), tn)
+		tableDesc, err = mustGetTableDesc(
+			ctx, txn, p.getVirtualTabler(), tn, false /*allowAdding*/)
 		return err
 	}); err != nil {
 		return nil, err

pkg/sql/table.go

@@ -158,9 +158,10 @@ func getViewDesc(
 }
 
 // mustGetTableOrViewDesc returns a table descriptor for either a table or
-// view, or an error if the descriptor is not found.
+// view, or an error if the descriptor is not found. allowAdding when set allows
+// a table descriptor in the ADD state to also be returned.
 func mustGetTableOrViewDesc(
-	ctx context.Context, txn *client.Txn, vt VirtualTabler, tn *parser.TableName,
+	ctx context.Context, txn *client.Txn, vt VirtualTabler, tn *parser.TableName, allowAdding bool,
 ) (*sqlbase.TableDescriptor, error) {
 	desc, err := getTableOrViewDesc(ctx, txn, vt, tn)
 	if err != nil {
@@ -170,15 +171,18 @@ func mustGetTableOrViewDesc(
 		return nil, sqlbase.NewUndefinedTableError(tn.String())
 	}
 	if err := filterTableState(desc); err != nil {
-		return nil, err
+		if !allowAdding && err != errTableAdding {
+			return nil, err
+		}
 	}
 	return desc, nil
 }
 
 // mustGetTableDesc returns a table descriptor for a table, or an error if
-// the descriptor is not found.
+// the descriptor is not found. allowAdding when set allows a table descriptor
+// in the ADD state to also be returned.
 func mustGetTableDesc(
-	ctx context.Context, txn *client.Txn, vt VirtualTabler, tn *parser.TableName,
+	ctx context.Context, txn *client.Txn, vt VirtualTabler, tn *parser.TableName, allowAdding bool,
 ) (*sqlbase.TableDescriptor, error) {
 	desc, err := getTableDesc(ctx, txn, vt, tn)
 	if err != nil {
@@ -188,13 +192,15 @@ func mustGetTableDesc(
 		return nil, sqlbase.NewUndefinedTableError(tn.String())
 	}
 	if err := filterTableState(desc); err != nil {
-		return nil, err
+		if !allowAdding && err != errTableAdding {
+			return nil, err
+		}
 	}
 	return desc, nil
 }
 
 // mustGetViewDesc returns a table descriptor for a view, or an error if the
-// descriptor is not found.
+// descriptor is not found or descriptor.Dropped().
 func mustGetViewDesc(
 	ctx context.Context, txn *client.Txn, vt VirtualTabler, tn *parser.TableName,
 ) (*sqlbase.TableDescriptor, error) {
@@ -246,13 +252,10 @@ func (lc *LeaseCollection) getTableLease(
 		//   so they cannot be leased. Instead, we simply return the static
 		//   descriptor and rely on the immutability privileges set on the
 		//   descriptors to cause upper layers to reject mutations statements.
-		tbl, err := mustGetTableDesc(ctx, txn, vt, tn)
+		tbl, err := mustGetTableDesc(ctx, txn, vt, tn, false /*allowAdding*/)
 		if err != nil {
 			return nil, err
 		}
-		if err := filterTableState(tbl); err != nil {
-			return nil, err
-		}
 		return tbl, nil
 	}
 
@@ -447,6 +450,12 @@ func (p *planner) writeTableDesc(ctx context.Context, tableDesc *sqlbase.TableDe
 	if isVirtualDescriptor(tableDesc) {
 		panic(fmt.Sprintf("Virtual Descriptors cannot be stored, found: %v", tableDesc))
 	}
+	// Some statements setTestingVerifyMetadata to verify the descriptor they
+	// have written, but if they are followed by other statements that modify
+	// the descriptor the verification of the overwritten descriptor cannot be
+	// done.
+	p.session.setTestingVerifyMetadata(nil)
+
 	return p.txn.Put(
 		ctx, sqlbase.MakeDescMetadataKey(tableDesc.GetID()), sqlbase.WrapDescriptor(tableDesc),
 	)
@@ -572,7 +581,9 @@ func (p *planner) findTableContainingIndex(
 	result = nil
 	for i := range tns {
 		tn := &tns[i]
-		tableDesc, err := mustGetTableDesc(ctx, p.txn, p.getVirtualTabler(), tn)
+		tableDesc, err := mustGetTableDesc(
+			ctx, p.txn, p.getVirtualTabler(), tn, true, /*allowAdding*/
+		)
 		if err != nil {
 			return nil, err
 		}

pkg/sql/testdata/logic_test/fk

@@ -675,7 +675,46 @@ statement ok
 BEGIN TRANSACTION
 
 statement ok
-CREATE TABLE refers (a INT REFERENCES referee);
+CREATE TABLE refers (
+  a INT REFERENCES referee,
+  b INT,
+  INDEX b_idx (b)
+)
+
+# Add some schema changes within the same transaction to verify that a
+# table that isn't yet public can be modified.
+statement ok
+CREATE INDEX foo ON refers (a)
+
+statement ok
+ALTER INDEX refers@b_idx RENAME TO another_idx
+
+query TT
+SHOW CREATE TABLE refers
+----
+refers  CREATE TABLE refers (
+a INT NULL,
+b INT NULL,
+INDEX another_idx (b ASC),
+CONSTRAINT fk_a_ref_referee FOREIGN KEY (a) REFERENCES referee (id),
+FAMILY "primary" (a, b, rowid)
+)
+
+statement ok
+DROP INDEX refers@another_idx
+
+statement ok
+COMMIT
+
+# CREATE AND DROP a table with a fk in the same transaction.
+statement ok
+BEGIN TRANSACTION
+
+statement ok
+CREATE TABLE refers1 (a INT REFERENCES referee);
+
+statement ok
+DROP TABLE refers1
 
 statement ok
 COMMIT