Merge pull request #74609 from nvanbenschoten/backport21.2-74338

release-21.2: sql: avoid string formatting in reportSessionDataChanges when not necessary
cockroachdb · Jan 10, 2022 · b71fb86 · b71fb86
2 parents ed50690 + 27fb118
commit b71fb86
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 5 deletions.
diff --git a/pkg/sql/conn_executor_exec.go b/pkg/sql/conn_executor_exec.go
@@ -844,15 +844,16 @@ func (ex *connExecutor) reportSessionDataChanges(fn func() error) error {
 			if err != nil {
 				return err
 			}
+			if v.Equal == nil {
+				return errors.AssertionFailedf("Equal for %s must be set", param.name)
+			}
 			if v.GetFromSessionData == nil {
 				return errors.AssertionFailedf("GetFromSessionData for %s must be set", param.name)
 			}
-			beforeVal := v.GetFromSessionData(before)
-			afterVal := v.GetFromSessionData(after)
-			if beforeVal != afterVal {
+			if !v.Equal(before, after) {
 				ex.dataMutatorIterator.paramStatusUpdater.BufferParamStatusUpdate(
 					param.name,
-					afterVal,
+					v.GetFromSessionData(after),
 				)
 			}
 		}

diff --git a/pkg/sql/vars.go b/pkg/sql/vars.go
@@ -106,6 +106,11 @@ type sessionVar struct {
 	// during session initialization when no default value was provided
 	// by the client.
 	GlobalDefault func(sv *settings.Values) string
+
+	// Equal returns whether the value of the given variable is equal between the
+	// two SessionData references. This is only required if the variable is
+	// expected to send updates through ParamStatusUpdate in pgwire.
+	Equal func(a, b *sessiondata.SessionData) bool
 }
 
 func formatBoolAsPostgresSetting(b bool) string {
@@ -161,7 +166,12 @@ var varGen = map[string]sessionVar{
 		GetFromSessionData: func(sd *sessiondata.SessionData) string {
 			return sd.ApplicationName
 		},
-		GlobalDefault: func(_ *settings.Values) string { return "" },
+		GlobalDefault: func(_ *settings.Values) string {
+			return ""
+		},
+		Equal: func(a, b *sessiondata.SessionData) bool {
+			return a.ApplicationName == b.ApplicationName
+		},
 	},
 
 	// See https://www.postgresql.org/docs/10/static/runtime-config-client.html
@@ -309,6 +319,9 @@ var varGen = map[string]sessionVar{
 		GlobalDefault: func(sv *settings.Values) string {
 			return dateStyleEnumMap[dateStyle.Get(sv)]
 		},
+		Equal: func(a, b *sessiondata.SessionData) bool {
+			return a.GetDateStyle() == b.GetDateStyle()
+		},
 	},
 	`datestyle_enabled`: {
 		Get: func(evalCtx *extendedEvalContext) string {
@@ -903,6 +916,9 @@ var varGen = map[string]sessionVar{
 		GlobalDefault: func(sv *settings.Values) string {
 			return strings.ToLower(duration.IntervalStyle_name[int32(intervalStyle.Get(sv))])
 		},
+		Equal: func(a, b *sessiondata.SessionData) bool {
+			return a.GetIntervalStyle() == b.GetIntervalStyle()
+		},
 	},
 	`intervalstyle_enabled`: {
 		Get: func(evalCtx *extendedEvalContext) string {
@@ -940,6 +956,9 @@ var varGen = map[string]sessionVar{
 		GlobalDefault: func(sv *settings.Values) string {
 			return "off"
 		},
+		Equal: func(a, b *sessiondata.SessionData) bool {
+			return a.IsSuperuser == b.IsSuperuser
+		},
 	},
 
 	// CockroachDB extension.
@@ -1252,6 +1271,10 @@ var varGen = map[string]sessionVar{
 		GetStringVal:  timeZoneVarGetStringVal,
 		Set:           timeZoneVarSet,
 		GlobalDefault: func(_ *settings.Values) string { return "UTC" },
+		Equal: func(a, b *sessiondata.SessionData) bool {
+			// NB: (*time.Location).String does not heap allocate.
+			return a.GetLocation().String() == b.GetLocation().String()
+		},
 	},
 
 	// This is not directly documented in PG's docs but does indeed behave this way.