sql: refactor render function for ShowCreateTrigger()

Moved `renderCreateTriggerStatement()` into
`show_trigger.go` and used it in `ShowCreateTrigger()`
to avoid repeating the code for generating trigger
create statements.
Also, removes the work-around explicitly
iterating over query, as the SQL query execution
bug is fixed.

Release note: None
Epic: CRDB-49582

Author: OriSavir

pkg/sql/conn_executor_exec.go

@@ -3458,8 +3458,7 @@ func (ex *connExecutor) makeExecPlan(
 			// session var is set to 'true' ('false' is the default).
 			catalog = planner.optPlanningCtx.catalog
 		}
-		planStr := ih.planGist.String()
-		_, err := explain.DecodePlanGistToRows(ctx, &planner.extendedEvalCtx.Context, planStr, catalog)
+		_, err := explain.DecodePlanGistToRows(ctx, &planner.extendedEvalCtx.Context, ih.planGist.String(), catalog)
 		if err != nil {
 			return ctx, errors.NewAssertionErrorWithWrappedErrf(err, "failed to decode plan gist: %q", ih.planGist.String())
 		}

pkg/sql/crdb_internal.go

@@ -3944,91 +3944,6 @@ CREATE TABLE crdb_internal.create_procedure_statements (
 	populate: createRoutinePopulate(true /* procedure */),
 }
 
-func renderCreateTriggerStatement(
-	ctx context.Context,
-	p *planner,
-	trigger *descpb.TriggerDescriptor,
-	tableDesc catalog.TableDescriptor,
-) (string, error) {
-	tableTyp, err := p.ResolveTypeByOID(ctx, typedesc.TableIDToImplicitTypeOID(tableDesc.GetID()))
-	if err != nil {
-		return "", err
-	}
-
-	funcName, err := p.GetQualifiedFunctionNameByID(ctx, int64(trigger.FuncID))
-	if err != nil {
-		return "", err
-	}
-
-	tableName, err := p.getQualifiedTableName(ctx, tableDesc)
-	if err != nil {
-		return "", err
-	}
-
-	events := make([]*tree.TriggerEvent, len(trigger.Events))
-	for j := range events {
-		descEvent := trigger.Events[j]
-		events[j] = &tree.TriggerEvent{
-			EventType: tree.TriggerEventTypeToTree[descEvent.Type],
-			Columns:   make(tree.NameList, 0, len(descEvent.ColumnNames)),
-		}
-		for _, colName := range descEvent.ColumnNames {
-			events[j].Columns = append(events[j].Columns, tree.Name(colName))
-		}
-	}
-
-	var transitions []*tree.TriggerTransition
-	if trigger.NewTransitionAlias != "" {
-		transitions = append(transitions, &tree.TriggerTransition{
-			IsNew: true,
-			Name:  tree.Name(trigger.NewTransitionAlias),
-		})
-	}
-	if trigger.OldTransitionAlias != "" {
-		transitions = append(transitions, &tree.TriggerTransition{
-			IsNew: false,
-			Name:  tree.Name(trigger.OldTransitionAlias),
-		})
-	}
-
-	forEach := tree.TriggerForEachStatement
-	if trigger.ForEachRow {
-		forEach = tree.TriggerForEachRow
-	}
-
-	var whenExpr tree.Expr
-	if trigger.WhenExpr != "" {
-		whenExpr, err = schemaexpr.ParseTriggerWhenExprForDisplay(
-			ctx, tableTyp, trigger.WhenExpr, p.EvalContext(), p.SemaCtx(), tree.FmtParsable,
-		)
-		if err != nil {
-			return "", err
-		}
-	}
-
-	createTrigger := &tree.CreateTrigger{
-		Replace:     false,
-		Name:        tree.Name(trigger.Name),
-		ActionTime:  tree.TriggerActionTimeToTree[trigger.ActionTime],
-		Events:      events,
-		TableName:   tableName.ToUnresolvedObjectName(),
-		Transitions: transitions,
-		ForEach:     forEach,
-		When:        whenExpr,
-		FuncName:    funcName.ToUnresolvedObjectName().ToUnresolvedName(),
-		FuncArgs:    trigger.FuncArgs,
-	}
-
-	f := tree.NewFmtCtx(
-		tree.FmtParsable,
-		tree.FmtDataConversionConfig(p.SessionData().DataConversionConfig),
-		tree.FmtLocation(p.SessionData().Location),
-	)
-	f.FormatNode(createTrigger)
-
-	return f.CloseAndGetString(), nil
-}
-
 func createTriggerPopulate(
 	ctx context.Context, p *planner, db catalog.DatabaseDescriptor, addRow func(...tree.Datum) error,
 ) error {

pkg/sql/sem/builtins/BUILD.bazel

@@ -24,7 +24,7 @@ go_library(
         "show_create_all_routines_builtin.go",
         "show_create_all_schemas_builtin.go",
         "show_create_all_tables_builtin.go",
-        "show_create_all_triggers.go",
+        "show_create_all_triggers_builtin.go",
         "show_create_all_types_builtin.go",
         "trigram_builtins.go",
         "tsearch_builtins.go",

pkg/sql/sem/builtins/generator_builtins.go

@@ -583,8 +583,7 @@ The output can be used to recreate a database.'
 			},
 			showCreateAllTriggersGeneratorType,
 			makeShowCreateAllTriggersGenerator,
-			`Returns rows of CREATE trigger statements.
-				The output can be used to recreate a database.`,
+			`Returns rows of CREATE trigger statements. The output can be used to recreate a database.`,
 			volatility.Volatile,
 		),
 	),
@@ -3097,7 +3096,7 @@ func makeShowCreateAllTablesGenerator(
 type showCreateAllTriggersGenerator struct {
 	evalPlanner eval.Planner
 	txn         *kv.Txn
-	ids         []triggerIdPair
+	ids         []tableTriggerPair
 	dbName      string
 	acc         mon.BoundAccount
 

pkg/sql/sem/builtins/show_create_all_triggers.go renamed to pkg/sql/sem/builtins/show_create_all_triggers_builtin.go

@@ -19,15 +19,14 @@ import (
 	"github.com/cockroachdb/errors"
 )
 
-type triggerIdPair struct {
+type tableTriggerPair struct {
 	triggerID int64
 	tableID   int64
 }
 
 func getTriggerIds(
 	ctx context.Context, evalPlanner eval.Planner, txn *kv.Txn, dbName string, acc *mon.BoundAccount,
-) (triggerIds []triggerIdPair, retErr error) { //TODO: Check up on ID field names etc
-	//TODO: Want to get (trigger_id, table_id)
+) (triggerIds []tableTriggerPair, retErr error) { //TODO: Check up on ID field names etc
 	query := fmt.Sprintf(`
 SELECT trigger_id, table_id 
 FROM %s.crdb_internal.create_trigger_statements 
@@ -50,7 +49,7 @@ WHERE database_name=$1`, lexbase.EscapeSQLIdent(dbName))
 	for ok, err = it.Next(ctx); ok; ok, err = it.Next(ctx) {
 		triggerID := tree.MustBeDInt(it.Cur()[0])
 		tableID := tree.MustBeDInt(it.Cur()[1])
-		pair := triggerIdPair{int64(triggerID), int64(tableID)}
+		pair := tableTriggerPair{int64(triggerID), int64(tableID)}
 		triggerIds = append(triggerIds, pair)
 		if err = acc.Grow(ctx, int64(unsafe.Sizeof(pair))); err != nil {
 			return nil, err
@@ -66,47 +65,27 @@ func getTriggerCreateStatement(
 	ctx context.Context,
 	evalPlanner eval.Planner,
 	txn *kv.Txn,
-	triggerIDPair triggerIdPair,
+	tableTriggerIds tableTriggerPair,
 	dbName string,
 ) (_ tree.Datum, err error) {
+	// Here, we query by `table_id` and 'trigger_id' but the query is only actually indexed by 'table_id'.
+	// This is because the `crdb_internal.create_trigger_statements` table only has a virtual index on `table_id`
+	// due to the fact that internal virtual tables do not support multi-column indexes
+	// and `trigger_id` values are only unique within a table, not between tables.
 	query := fmt.Sprintf(`
 SELECT create_statement, trigger_id
 FROM %s.crdb_internal.create_trigger_statements
-WHERE table_id = $1`, lexbase.EscapeSQLIdent(dbName))
+WHERE table_id = $1 AND trigger_id=$2`, lexbase.EscapeSQLIdent(dbName))
 
-	iter, err := evalPlanner.QueryIteratorEx(ctx,
+	row, err := evalPlanner.QueryRowEx(
+		ctx,
 		"crdb_internal.show_create_all_triggers",
 		sessiondata.NoSessionDataOverride,
 		query,
-		triggerIDPair.tableID)
+		tableTriggerIds.tableID, tableTriggerIds.triggerID)
 	if err != nil {
 		return nil, err
 	}
 
-	defer func() {
-		closeErr := iter.Close()
-		err = errors.CombineErrors(err, closeErr)
-	}()
-	for {
-		next, err := iter.Next(ctx)
-		if err != nil {
-			return nil, err
-		}
-		if !next {
-			break
-		}
-		row := iter.Cur()
-		if len(row) != 2 {
-			return nil, errors.Newf("expected 2 columns in result, got %d", len(row))
-		}
-		if tree.MustBeDInt(row[1]) != tree.DInt(triggerIDPair.triggerID) {
-			continue
-		}
-		return row[0], nil
-	}
-
-	if err != nil {
-		return nil, err
-	}
-	return nil, nil
+	return row[0], nil
 }