diff --git a/pkg/sql/alter_role.go b/pkg/sql/alter_role.go index 9ca6463a0b37..b712d41f44db 100644 --- a/pkg/sql/alter_role.go +++ b/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) } diff --git a/pkg/sql/sem/tree/type_check.go b/pkg/sql/sem/tree/type_check.go index 6b357457d3cf..4831861e5ead 100644 --- a/pkg/sql/sem/tree/type_check.go +++ b/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] diff --git a/pkg/sql/set_var.go b/pkg/sql/set_var.go index 16849c4b3763..7d3c0cd16d23 100644 --- a/pkg/sql/set_var.go +++ b/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) { diff --git a/pkg/sql/vars.go b/pkg/sql/vars.go index 16de1f236186..97b0e66f4af3 100644 --- a/pkg/sql/vars.go +++ b/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.