sql: rework some internal executor logic related to synthetic descrip…

…otrs

The pre-existing logic would reset the synthetic descriptors whenever anything
was run on behalf of the internal planner. That's never what we want. We
prevent that by attaching the logic to reset the synthetic descriptors to
whether the internal executor has attached synthetic descriptors, which
it never has when using the planner's state.

Release justification: part of bug fix

Release note: None

pkg/sql/conn_executor.go

@@ -1246,6 +1246,10 @@ type connExecutor struct {
 		// delete schema change job records. Instead, we leave the caller of the
 		// internal executor to release them.
 		fromOuterTxn bool
+		// shouldResetSyntheticDescriptors should be set to true only if
+		// fromOuterTxn is set to true, and, upon finishing the statement, the
+		// synthetic descriptors should be reset.
+		shouldResetSyntheticDescriptors bool
 
 		// descCollection collects descriptors used by the current transaction.
 		descCollection *descs.Collection
@@ -1662,7 +1666,9 @@ func (ex *connExecutor) resetExtraTxnState(ctx context.Context, ev txnEvent) {
 	ex.extraTxnState.hasAdminRoleCache = HasAdminRoleCache{}
 
 	if ex.extraTxnState.fromOuterTxn {
-		ex.extraTxnState.descCollection.ResetSyntheticDescriptors()
+		if ex.extraTxnState.shouldResetSyntheticDescriptors {
+			ex.extraTxnState.descCollection.ResetSyntheticDescriptors()
+		}
 	} else {
 		ex.extraTxnState.descCollection.ReleaseAll(ctx)
 		for k := range ex.extraTxnState.schemaChangeJobRecords {
@@ -3267,7 +3273,7 @@ func (ex *connExecutor) runPreCommitStages(ctx context.Context) error {
 		scs.jobID,
 		scs.stmts,
 	)
-
+	ex.extraTxnState.descCollection.ResetSyntheticDescriptors()
 	after, jobID, err := scrun.RunPreCommitPhase(
 		ctx, ex.server.cfg.DeclarativeSchemaChangerTestingKnobs, deps, scs.state,
 	)

pkg/sql/internal.go

@@ -331,14 +331,14 @@ func (ie *InternalExecutor) newConnExecutorWithTxn(
 	// let it inherit the descriptor collection, schema change job records
 	// and job collections from the caller.
 	postSetupFn := func(ex *connExecutor) {
-		if ie.extraTxnState != nil {
-			if ie.extraTxnState.descCollection != nil {
-				ex.extraTxnState.descCollection = ie.extraTxnState.descCollection
-				ex.extraTxnState.fromOuterTxn = true
-				ex.extraTxnState.schemaChangeJobRecords = ie.extraTxnState.schemaChangeJobRecords
-				ex.extraTxnState.jobs = ie.extraTxnState.jobs
-				ex.extraTxnState.schemaChangerState = ie.extraTxnState.schemaChangerState
-			}
+		if ie.extraTxnState != nil && ie.extraTxnState.descCollection != nil {
+			ex.extraTxnState.descCollection = ie.extraTxnState.descCollection
+			ex.extraTxnState.fromOuterTxn = true
+			ex.extraTxnState.shouldResetSyntheticDescriptors = len(ie.syntheticDescriptors) > 0
+			ex.extraTxnState.schemaChangeJobRecords = ie.extraTxnState.schemaChangeJobRecords
+			ex.extraTxnState.jobs = ie.extraTxnState.jobs
+			ex.extraTxnState.schemaChangerState = ie.extraTxnState.schemaChangerState
+			ex.initPlanner(ctx, &ex.planner)
 		}
 	}
 
@@ -393,7 +393,9 @@ func (ie *InternalExecutor) newConnExecutorWithTxn(
 	// Modify the Collection to match the parent executor's Collection.
 	// This allows the InternalExecutor to see schema changes made by the
 	// parent executor.
-	ex.extraTxnState.descCollection.SetSyntheticDescriptors(ie.syntheticDescriptors)
+	if len(ie.syntheticDescriptors) > 0 {
+		ex.extraTxnState.descCollection.SetSyntheticDescriptors(ie.syntheticDescriptors)
+	}
 	return ex, err
 }