From efdaecee060d523124a5a94ba9eb95ea8e72f59b Mon Sep 17 00:00:00 2001
From: Yahor Yuzefovich <yahor@cockroachlabs.com>
Date: Mon, 9 Jan 2023 11:38:36 -0800
Subject: [PATCH] sql: propagate the settings properly into the stmt bundle
 builder

A few months ago we extended the collection of the cluster settings into
the stmt bundle to also get the global default value. That getter method
takes in a pointer to the current cluster settings' values, and we
passed nil. This can cause nil pointer errors and is now fixed.

Release note: None
---
 pkg/sql/explain_bundle.go   | 13 ++++++++-----
 pkg/sql/instrumentation.go  |  2 +-
 pkg/sql/opt_exec_factory.go |  2 +-
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/pkg/sql/explain_bundle.go b/pkg/sql/explain_bundle.go
index ce3507930f38..bcb625f8fd93 100644
--- a/pkg/sql/explain_bundle.go
+++ b/pkg/sql/explain_bundle.go
@@ -132,11 +132,12 @@ func buildStatementBundle(
 	trace tracingpb.Recording,
 	placeholders *tree.PlaceholderInfo,
 	queryErr, payloadErr, commErr error,
+	sv *settings.Values,
 ) diagnosticsBundle {
 	if plan == nil {
 		return diagnosticsBundle{collectionErr: errors.AssertionFailedf("execution terminated early")}
 	}
-	b := makeStmtBundleBuilder(db, ie, stmtRawSQL, plan, trace, placeholders)
+	b := makeStmtBundleBuilder(db, ie, stmtRawSQL, plan, trace, placeholders, sv)
 
 	b.addStatement()
 	b.addOptPlans()
@@ -194,6 +195,7 @@ type stmtBundleBuilder struct {
 	plan         *planTop
 	trace        tracingpb.Recording
 	placeholders *tree.PlaceholderInfo
+	sv           *settings.Values
 
 	z memzipper.Zipper
 }
@@ -205,9 +207,10 @@ func makeStmtBundleBuilder(
 	plan *planTop,
 	trace tracingpb.Recording,
 	placeholders *tree.PlaceholderInfo,
+	sv *settings.Values,
 ) stmtBundleBuilder {
 	b := stmtBundleBuilder{
-		db: db, ie: ie, stmt: stmt, plan: plan, trace: trace, placeholders: placeholders,
+		db: db, ie: ie, stmt: stmt, plan: plan, trace: trace, placeholders: placeholders, sv: sv,
 	}
 	b.buildPrettyStatement()
 	b.z.Init()
@@ -364,7 +367,7 @@ func (b *stmtBundleBuilder) addEnv(ctx context.Context) {
 	fmt.Fprintf(&buf, "\n")
 
 	// Show the values of session variables that can impact planning decisions.
-	if err := c.PrintSessionSettings(&buf); err != nil {
+	if err := c.PrintSessionSettings(&buf, b.sv); err != nil {
 		fmt.Fprintf(&buf, "-- error getting session settings: %v\n", err)
 	}
 
@@ -565,7 +568,7 @@ func (c *stmtEnvCollector) PrintVersion(w io.Writer) error {
 
 // PrintSessionSettings appends information about session settings that can
 // impact planning decisions.
-func (c *stmtEnvCollector) PrintSessionSettings(w io.Writer) error {
+func (c *stmtEnvCollector) PrintSessionSettings(w io.Writer, sv *settings.Values) error {
 	// Cluster setting encoded default value to session setting value conversion
 	// functions.
 	boolToOnOff := func(boolStr string) string {
@@ -666,7 +669,7 @@ func (c *stmtEnvCollector) PrintSessionSettings(w io.Writer) error {
 		if s.clusterSetting == nil {
 			if ok, v, _ := getSessionVar(s.sessionSetting, true); ok {
 				if v.GlobalDefault != nil {
-					def = v.GlobalDefault(nil /* *settings.Values */)
+					def = v.GlobalDefault(sv)
 				}
 			}
 		} else {
diff --git a/pkg/sql/instrumentation.go b/pkg/sql/instrumentation.go
index 79c4bff2dc18..753cb9e8f501 100644
--- a/pkg/sql/instrumentation.go
+++ b/pkg/sql/instrumentation.go
@@ -398,7 +398,7 @@ func (ih *instrumentationHelper) Finish(
 			}
 			bundle = buildStatementBundle(
 				ih.origCtx, cfg.DB, ie.(*InternalExecutor), stmtRawSQL, &p.curPlan, ob.BuildString(), trace,
-				placeholders, res.Err(), payloadErr, retErr,
+				placeholders, res.Err(), payloadErr, retErr, &p.extendedEvalCtx.Settings.SV,
 			)
 			bundle.insert(
 				ctx, ih.fingerprint, ast, cfg.StmtDiagnosticsRecorder, ih.diagRequestID, ih.diagRequest,
diff --git a/pkg/sql/opt_exec_factory.go b/pkg/sql/opt_exec_factory.go
index 4bd6092ccb71..9f4d542c5238 100644
--- a/pkg/sql/opt_exec_factory.go
+++ b/pkg/sql/opt_exec_factory.go
@@ -1212,7 +1212,7 @@ func (ef *execFactory) showEnv(plan string, envOpts exec.ExplainEnvData) (exec.N
 	out.writef("")
 	// Show the values of any non-default session variables that can impact
 	// planning decisions.
-	if err := c.PrintSessionSettings(&out.buf); err != nil {
+	if err := c.PrintSessionSettings(&out.buf, &ef.planner.extendedEvalCtx.Settings.SV); err != nil {
 		return nil, err
 	}