Skip to content

Commit 28a65fc

Browse files
tglsfdctglsfdc
committed
Include GUC's unit, if it has one, in out-of-range error messages.
This should reduce confusion in cases where we've applied a units conversion, so that the number being reported (and the quoted range limits) are in some other units than what the user gave in the setting we're rejecting. Some of the changes here assume that float GUCs can have units, which isn't true just yet, but will be shortly. Discussion: https://postgr.es/m/3811.1552169665@sss.pgh.pa.us
1 parent ac75959 commit 28a65fc

File tree

2 files changed

+66
-49
lines changed

2 files changed

+66
-49
lines changed

‎src/backend/utils/misc/guc.c

+65-48
Original file line numberDiff line numberDiff line change
@@ -6038,6 +6038,54 @@ convert_from_base_unit(int64 base_value, int base_unit,
60386038
Assert(*unit != NULL);
60396039
}
60406040

6041+
/*
6042+
* Return the name of a GUC's base unit (e.g. "ms") given its flags.
6043+
* Return NULL if the GUC is unitless.
6044+
*/
6045+
static const char *
6046+
get_config_unit_name(int flags)
6047+
{
6048+
switch (flags & (GUC_UNIT_MEMORY | GUC_UNIT_TIME))
6049+
{
6050+
case 0:
6051+
return NULL; /* GUC has no units */
6052+
case GUC_UNIT_BYTE:
6053+
return "B";
6054+
case GUC_UNIT_KB:
6055+
return "kB";
6056+
case GUC_UNIT_MB:
6057+
return "MB";
6058+
case GUC_UNIT_BLOCKS:
6059+
{
6060+
static char bbuf[8];
6061+
6062+
/* initialize if first time through */
6063+
if (bbuf[0] == '\0')
6064+
snprintf(bbuf, sizeof(bbuf), "%dkB", BLCKSZ / 1024);
6065+
return bbuf;
6066+
}
6067+
case GUC_UNIT_XBLOCKS:
6068+
{
6069+
static char xbuf[8];
6070+
6071+
/* initialize if first time through */
6072+
if (xbuf[0] == '\0')
6073+
snprintf(xbuf, sizeof(xbuf), "%dkB", XLOG_BLCKSZ / 1024);
6074+
return xbuf;
6075+
}
6076+
case GUC_UNIT_MS:
6077+
return "ms";
6078+
case GUC_UNIT_S:
6079+
return "s";
6080+
case GUC_UNIT_MIN:
6081+
return "min";
6082+
default:
6083+
elog(ERROR, "unrecognized GUC units value: %d",
6084+
flags & (GUC_UNIT_MEMORY | GUC_UNIT_TIME));
6085+
return NULL;
6086+
}
6087+
}
6088+
60416089

60426090
/*
60436091
* Try to parse value as an integer. The accepted formats are the
@@ -6324,10 +6372,15 @@ parse_and_validate_value(struct config_generic *record,
63246372

63256373
if (newval->intval < conf->min || newval->intval > conf->max)
63266374
{
6375+
const char *unit = get_config_unit_name(conf->gen.flags);
6376+
63276377
ereport(elevel,
63286378
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
6329-
errmsg("%d is outside the valid range for parameter \"%s\" (%d .. %d)",
6330-
newval->intval, name,
6379+
errmsg("%d%s%s is outside the valid range for parameter \"%s\" (%d .. %d)",
6380+
newval->intval,
6381+
unit ? " " : "",
6382+
unit ? unit : "",
6383+
name,
63316384
conf->min, conf->max)));
63326385
return false;
63336386
}
@@ -6352,10 +6405,15 @@ parse_and_validate_value(struct config_generic *record,
63526405

63536406
if (newval->realval < conf->min || newval->realval > conf->max)
63546407
{
6408+
const char *unit = get_config_unit_name(conf->gen.flags);
6409+
63556410
ereport(elevel,
63566411
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
6357-
errmsg("%g is outside the valid range for parameter \"%s\" (%g .. %g)",
6358-
newval->realval, name,
6412+
errmsg("%g%s%s is outside the valid range for parameter \"%s\" (%g .. %g)",
6413+
newval->realval,
6414+
unit ? " " : "",
6415+
unit ? unit : "",
6416+
name,
63596417
conf->min, conf->max)));
63606418
return false;
63616419
}
@@ -8687,52 +8745,11 @@ GetConfigOptionByNum(int varnum, const char **values, bool *noshow)
86878745
/* name */
86888746
values[0] = conf->name;
86898747

8690-
/* setting : use _ShowOption in order to avoid duplicating the logic */
8748+
/* setting: use _ShowOption in order to avoid duplicating the logic */
86918749
values[1] = _ShowOption(conf, false);
86928750

8693-
/* unit */
8694-
if (conf->vartype == PGC_INT)
8695-
{
8696-
switch (conf->flags & (GUC_UNIT_MEMORY | GUC_UNIT_TIME))
8697-
{
8698-
case GUC_UNIT_BYTE:
8699-
values[2] = "B";
8700-
break;
8701-
case GUC_UNIT_KB:
8702-
values[2] = "kB";
8703-
break;
8704-
case GUC_UNIT_MB:
8705-
values[2] = "MB";
8706-
break;
8707-
case GUC_UNIT_BLOCKS:
8708-
snprintf(buffer, sizeof(buffer), "%dkB", BLCKSZ / 1024);
8709-
values[2] = pstrdup(buffer);
8710-
break;
8711-
case GUC_UNIT_XBLOCKS:
8712-
snprintf(buffer, sizeof(buffer), "%dkB", XLOG_BLCKSZ / 1024);
8713-
values[2] = pstrdup(buffer);
8714-
break;
8715-
case GUC_UNIT_MS:
8716-
values[2] = "ms";
8717-
break;
8718-
case GUC_UNIT_S:
8719-
values[2] = "s";
8720-
break;
8721-
case GUC_UNIT_MIN:
8722-
values[2] = "min";
8723-
break;
8724-
case 0:
8725-
values[2] = NULL;
8726-
break;
8727-
default:
8728-
elog(ERROR, "unrecognized GUC units value: %d",
8729-
conf->flags & (GUC_UNIT_MEMORY | GUC_UNIT_TIME));
8730-
values[2] = NULL;
8731-
break;
8732-
}
8733-
}
8734-
else
8735-
values[2] = NULL;
8751+
/* unit, if any (NULL is fine) */
8752+
values[2] = get_config_unit_name(conf->flags);
87368753

87378754
/* group */
87388755
values[3] = _(config_group_names[conf->group]);

‎src/test/regress/expected/guc.out

+1-1
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,7 @@ SELECT '2006-08-13 12:34:56'::timestamptz;
510510
SET seq_page_cost TO 'NaN';
511511
ERROR: parameter "seq_page_cost" requires a numeric value
512512
SET vacuum_cost_delay TO '10s';
513-
ERROR: 10000 is outside the valid range for parameter "vacuum_cost_delay" (0 .. 100)
513+
ERROR: 10000 ms is outside the valid range for parameter "vacuum_cost_delay" (0 .. 100)
514514
SET geqo_selection_bias TO '-infinity';
515515
ERROR: -Infinity is outside the valid range for parameter "geqo_selection_bias" (1.5 .. 2)
516516
--

0 commit comments

Comments
 (0)