sql: address some cases of flattened error objects

There are some cases of error objects being flattened, which
strips them of all detail such as stack traces and user-directed
hints. This change address a few of the cases by wrapping the
error object.

Release note: None

pkg/sql/alter_role.go

@@ -432,7 +432,7 @@ func (p *planner) processSetOrResetClause(
 			ctx, expr, nil, tree.IndexedVarHelper{}, types.String, false, "ALTER ROLE ... SET ",
 		)
 		if err != nil {
-			return unknown, "", sessionVar{}, nil, wrapSetVarError(varName, expr.String(), "%v", err)
+			return unknown, "", sessionVar{}, nil, wrapSetVarError(err, varName, expr.String())
 		}
 		typedValues = append(typedValues, typedValue)
 	}

pkg/sql/sem/tree/type_check.go

@@ -2398,8 +2398,8 @@ func typeCheckTupleComparison(
 		leftSubExprTyped, rightSubExprTyped, _, _, err := typeCheckComparisonOp(ctx, semaCtx, op, leftSubExpr, rightSubExpr)
 		if err != nil {
 			exps := Exprs([]Expr{left, right})
-			return nil, nil, pgerror.Newf(pgcode.DatatypeMismatch, "tuples %s are not comparable at index %d: %s",
-				&exps, elemIdx+1, err)
+			return nil, nil, pgerror.Wrapf(err, pgcode.DatatypeMismatch, "tuples %s are not comparable at index %d",
+				&exps, elemIdx+1)
 		}
 		left.Exprs[elemIdx] = leftSubExprTyped
 		left.typ.TupleContents()[elemIdx] = leftSubExprTyped.ResolvedType()
@@ -2457,7 +2457,7 @@ func typeCheckSameTypedTupleExprs(
 		}
 		typedSubExprs, resType, err := TypeCheckSameTypedExprs(ctx, semaCtx, desiredElem, sameTypeExprs...)
 		if err != nil {
-			return nil, nil, pgerror.Newf(pgcode.DatatypeMismatch, "tuples %s are not the same type: %v", Exprs(exprs), err)
+			return nil, nil, pgerror.Wrapf(err, pgcode.DatatypeMismatch, "tuples %s are not the same type", Exprs(exprs))
 		}
 		for j, typedExpr := range typedSubExprs {
 			tupleIdx := sameTypeExprsIndices[j]

pkg/sql/set_var.go

@@ -27,6 +27,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/duration"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/redact"
 )
 
 // setVarNode represents a SET {SESSION | LOCAL} statement.
@@ -76,7 +77,7 @@ func (p *planner) SetVar(ctx context.Context, n *tree.SetVar) (planNode, error)
 				typedValue, err := p.analyzeExpr(
 					ctx, expr, nil, dummyHelper, types.String, false, "SET SESSION "+name)
 				if err != nil {
-					return nil, wrapSetVarError(name, expr.String(), "%v", err)
+					return nil, wrapSetVarError(err, name, expr.String())
 				}
 				typedValues[i] = typedValue
 			}
@@ -223,14 +224,13 @@ func timeZoneVarGetStringVal(
 			timeutil.TimeZoneStringToLocationISO8601Standard,
 		)
 		if err != nil {
-			return "", wrapSetVarError("timezone", values[0].String(),
-				"cannot find time zone %q: %v", location, err)
+			return "", wrapSetVarError(errors.Wrapf(err, "cannot find time zone %q", location), "timezone", values[0].String())
 		}
 
 	case *tree.DInterval:
 		offset, _, _, err = v.Duration.Encode()
 		if err != nil {
-			return "", wrapSetVarError("timezone", values[0].String(), "%v", err)
+			return "", wrapSetVarError(err, "timezone", values[0].String())
 		}
 		offset /= int64(time.Second)
 
@@ -247,8 +247,7 @@ func timeZoneVarGetStringVal(
 		ed.Mul(sixty, sixty, &v.Decimal)
 		offset = ed.Int64(sixty)
 		if ed.Err() != nil {
-			return "", wrapSetVarError("timezone", values[0].String(),
-				"time zone value %s would overflow an int64", sixty)
+			return "", wrapSetVarError(errors.Newf("time zone value %s would overflow an int64", sixty), "timezone", values[0].String())
 		}
 
 	default:
@@ -267,7 +266,7 @@ func timeZoneVarSet(_ context.Context, m sessionDataMutator, s string) error {
 		timeutil.TimeZoneStringToLocationISO8601Standard,
 	)
 	if err != nil {
-		return wrapSetVarError("TimeZone", s, "%v", err)
+		return wrapSetVarError(err, "TimeZone", s)
 	}
 
 	m.SetLocation(loc)
@@ -296,7 +295,7 @@ func makeTimeoutVarGetter(
 		case *tree.DInterval:
 			timeout, err = intervalToDuration(v)
 			if err != nil {
-				return "", wrapSetVarError(varName, values[0].String(), "%v", err)
+				return "", wrapSetVarError(err, varName, values[0].String())
 			}
 		case *tree.DInt:
 			timeout = time.Duration(*v) * time.Millisecond
@@ -318,16 +317,15 @@ func validateTimeoutVar(
 		},
 	)
 	if err != nil {
-		return 0, wrapSetVarError(varName, timeString, "%v", err)
+		return 0, wrapSetVarError(err, varName, timeString)
 	}
 	timeout, err := intervalToDuration(interval)
 	if err != nil {
-		return 0, wrapSetVarError(varName, timeString, "%v", err)
+		return 0, wrapSetVarError(err, varName, timeString)
 	}
 
 	if timeout < 0 {
-		return 0, wrapSetVarError(varName, timeString,
-			"%v cannot have a negative duration", varName)
+		return 0, wrapSetVarError(errors.Newf("%v cannot have a negative duration", redact.SafeString(varName)), varName, timeString)
 	}
 
 	return timeout, nil
@@ -404,10 +402,14 @@ func newSingleArgVarError(varName string) error {
 		"SET %s takes only one argument", varName)
 }
 
-func wrapSetVarError(varName, actualValue string, fmt string, args ...interface{}) error {
-	err := pgerror.Newf(pgcode.InvalidParameterValue,
-		"invalid value for parameter %q: %q", varName, actualValue)
-	return errors.WithDetailf(err, fmt, args...)
+func wrapSetVarError(cause error, varName, actualValue string) error {
+	return pgerror.Wrapf(
+		cause,
+		pgcode.InvalidParameterValue,
+		"invalid value for parameter %q: %q",
+		redact.SafeString(varName),
+		actualValue,
+	)
 }
 
 func newVarValueError(varName, actualVal string, allowedVals ...string) (err error) {

pkg/sql/vars.go

@@ -339,7 +339,7 @@ var varGen = map[string]sessionVar{
 		Set: func(ctx context.Context, m sessionDataMutator, val string) error {
 			i, err := strconv.ParseInt(val, 10, 64)
 			if err != nil {
-				return wrapSetVarError("default_int_size", val, "%v", err)
+				return wrapSetVarError(err, "default_int_size", val)
 			}
 			if i != 4 && i != 8 {
 				return pgerror.New(pgcode.InvalidParameterValue,
@@ -813,7 +813,7 @@ var varGen = map[string]sessionVar{
 		) error {
 			i, err := strconv.ParseInt(s, 10, 64)
 			if err != nil {
-				return wrapSetVarError("extra_float_digits", s, "%v", err)
+				return wrapSetVarError(err, "extra_float_digits", s)
 			}
 			// Note: this is the range allowed by PostgreSQL.
 			// See also the documentation around (DataConversionConfig).GetFloatPrec()
@@ -1113,7 +1113,7 @@ var varGen = map[string]sessionVar{
 		Set: func(_ context.Context, _ sessionDataMutator, s string) error {
 			i, err := strconv.ParseInt(s, 10, 64)
 			if err != nil {
-				return wrapSetVarError("ssl_renegotiation_limit", s, "%v", err)
+				return wrapSetVarError(err, "ssl_renegotiation_limit", s)
 			}
 			if i != 0 {
 				// See pg src/backend/utils/misc/guc.c: non-zero values are not to be supported.