Merge #142748

142748: sql: fix up recent stmt bundle change r=yuzefovich a=yuzefovich

Epic: None

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: craig[bot]

pkg/sql/explain_bundle.go

@@ -628,19 +628,19 @@ func (b *stmtBundleBuilder) addEnv(ctx context.Context) {
 			ctx,
 			b.plan.catalog,
 			mem.Metadata().AllTables(),
-			func(table cat.Table, fk cat.ForeignKeyConstraint) (exploreFKs bool) {
+			func(table cat.Table, fk cat.ForeignKeyConstraint) (recurse bool) {
 				if includeAll {
 					return true
 				}
 				if !hasMutation {
 					// For read-only queries, we don't care about any tables not
-					// referenced by metadata, so we don't want to explore any
-					// FKs.
+					// referenced by metadata, so we don't want to recursed into
+					// any FKs.
 					return false
 				}
 				if referencedByMetadata.Contains(int(table.ID())) || fk == nil {
-					// For mutations, we always want to explore FKs of tables
-					// referenced by metadata.
+					// For mutations, we always want to recurse into FKs of
+					// tables referenced by metadata.
 					//
 					// The second part of the conditional should never evaluate
 					// to 'true' since nil FK parameter is provided only for
@@ -654,31 +654,33 @@ func (b *stmtBundleBuilder) addEnv(ctx context.Context) {
 				//   CREATE TABLE child (pk INT PRIMARY KEY, fk INT REFERENCES parent(pk));
 				//   CREATE TABLE grandchild (pk INT PRIMARY KEY, fk INT REFERENCES child(pk));
 				if table.ID() == fk.ReferencedTableID() {
-					// The table we're considering for exploration is a
-					// referenced table of a FK constraint where the referencing
-					// (origin) table has already been visited.
+					// The table we're considering for recursion is a referenced
+					// table of a FK constraint where the referencing (origin)
+					// table has already been visited.
 					//
 					// In our example, we've already visited 'child' and are
-					// considering exploring 'parent', but we never actually
-					// want to explore it.
+					// considering recursing into 'parent', but we never
+					// actually want to do that.
 					return false
 				}
-				// The table we're considering for exploration is an origin
+				// The table we're considering for recursion is an origin
 				// table of a FK constraint where the referenced table has
 				// already been visited.
 				//
 				// In our example, we've already visited 'parent' and are
-				// considering exploring 'child'.
+				// considering recursing into 'child'.
 				if hasDelete && fk.DeleteReferenceAction() == tree.Cascade {
 					// We deleted from 'parent', and we have the ON DELETE
-					// CASCADE action of the 'child' FK, so we need to explore
-					// child's FKs in order to additionally visit 'grandchild'.
+					// CASCADE action of the 'child' FK, so we need to recurse
+					// into child's FKs in order to additionally visit
+					// 'grandchild'.
 					return true
 				}
 				if (hasUpdate || hasUpsert) && fk.UpdateReferenceAction() == tree.Cascade {
 					// We updated the 'parent', and we have the ON UPDATE
-					// CASCADE action of the 'child' FK, so we need to explore
-					// child's FKs in order to additionally visit 'grandchild'.
+					// CASCADE action of the 'child' FK, so we need to recurse
+					// into child's FKs in order to additionally visit
+					// 'grandchild'.
 					//
 					// We don't know whether UPSERT resulted in an UPDATE or an
 					// INSERT, but we'll assume the former to be on the safer

pkg/sql/opt/exec/execbuilder/relational.go

@@ -4164,7 +4164,7 @@ func (b *Builder) getEnvData() (exec.ExplainEnvData, error) {
 		b.ctx,
 		b.catalog,
 		b.mem.Metadata().AllTables(),
-		func(cat.Table, cat.ForeignKeyConstraint) (exploreFKs bool) {
+		func(cat.Table, cat.ForeignKeyConstraint) (recurse bool) {
 			return true
 		},
 		func(table cat.Table, _ cat.ForeignKeyConstraint) {

pkg/sql/opt/util.go

@@ -18,9 +18,8 @@ import (
 // are visited in sorted order so that later tables reference earlier tables.
 //
 // The visiting is controlled by two callbacks:
-// - visitPreFn should tell the visitor whether the FKs of the given tables
-// should be explored (i.e. whether the visitor should "recurse" into FK
-// reference tables of the given one).
+// - recurseFn should tell the visitor whether it should recurse into FK
+// reference tables of the given one.
 // - visitFn allows the caller to do any work on the table being visited.
 //
 // In both functions:
@@ -35,39 +34,39 @@ func VisitFKReferenceTables(
 	ctx context.Context,
 	catalog cat.Catalog,
 	tables []TableMeta,
-	visitPreFn func(_ cat.Table, fk cat.ForeignKeyConstraint) (exploreFKs bool),
+	recurseFn func(_ cat.Table, fk cat.ForeignKeyConstraint) bool,
 	visitFn func(_ cat.Table, fk cat.ForeignKeyConstraint),
 ) {
-	// tableExplored tracks which tables we've already explored FKs of. Once a
-	// table is explored, it is considered "fully processed" and we effectively
-	// ignore it from now on. If a table has already been visited but is not
-	// explored, we still might want to explore it later (because we might get
-	// to it via a different FK that requires exploration).
-	var tableExplored intsets.Fast
+	// tableRecursed tracks which tables we've already recursed into. Once a
+	// table is recursed into, it is considered "fully processed" and we
+	// effectively ignore it from now on. If a table has already been visited
+	// but is not recursed into, we still might want to recurse into it later
+	// (because we might get to it via a different FK that requires recursion).
+	var tableRecursed intsets.Fast
 	var visitForeignKeyReferencedTables func(tab cat.Table)
 	var visitForeignKeyReferencingTables func(tab cat.Table)
-	visitTable := func(table cat.Table, fk cat.ForeignKeyConstraint, exploreFKs bool) {
+	visitTable := func(table cat.Table, fk cat.ForeignKeyConstraint, recurse bool) {
 		tabID := table.ID()
-		if exploreFKs {
-			tableExplored.Add(int(tabID))
+		if recurse {
+			tableRecursed.Add(int(tabID))
 		}
 		// The order of visiting here is important: namely, we want to visit
 		// all tables that we reference first, then ourselves, and only then
 		// tables that reference us.
-		if exploreFKs {
+		if recurse {
 			visitForeignKeyReferencedTables(table)
 		}
 		visitFn(table, fk)
-		if exploreFKs {
+		if recurse {
 			visitForeignKeyReferencingTables(table)
 		}
 	}
 	// handleRelatedTables is a helper function that processes the given table
-	// if it hasn't been explored yet by visiting all referenced and referencing
-	// table of the given one, including via transient (recursive) FK
-	// relationships.
+	// if it hasn't been recursed into yet by visiting all referenced and
+	// referencing table of the given one, including via transient (recursive)
+	// FK relationships.
 	handleRelatedTables := func(tabID cat.StableID, fk cat.ForeignKeyConstraint) {
-		if !tableExplored.Contains(int(tabID)) {
+		if !tableRecursed.Contains(int(tabID)) {
 			ds, _, err := catalog.ResolveDataSourceByID(ctx, cat.Flags{}, tabID)
 			if err != nil {
 				// This is a best-effort attempt to get all the tables, so don't
@@ -80,8 +79,8 @@ func VisitFKReferenceTables(
 				// error.
 				return
 			}
-			exploreFKs := visitPreFn(refTab, fk)
-			visitTable(refTab, fk, exploreFKs)
+			recurse := recurseFn(refTab, fk)
+			visitTable(refTab, fk, recurse)
 		}
 	}
 	visitForeignKeyReferencedTables = func(tab cat.Table) {
@@ -98,9 +97,9 @@ func VisitFKReferenceTables(
 	}
 	for _, tabMeta := range tables {
 		tabID := tabMeta.Table.ID()
-		if !tableExplored.Contains(int(tabID)) {
-			exploreFKs := visitPreFn(tabMeta.Table, nil /* fk */)
-			visitTable(tabMeta.Table, nil /* fk */, exploreFKs)
+		if !tableRecursed.Contains(int(tabID)) {
+			recurse := recurseFn(tabMeta.Table, nil /* fk */)
+			visitTable(tabMeta.Table, nil /* fk */, recurse)
 		}
 	}
 }