Merge pull request #15929 from vivekmenezes/fk

sql: allow schema changes on tables in state: TableDescriptor_ADD

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