Improve ereports for VACUUM's BUFFER_USAGE_LIMIT option

There's no need to check if opt->arg is NULL since defGetString() already
does that and raises an ERROR if it is.  Let's just remove that check.

Also, combine the two remaining ERRORs into a single check.  It seems
better to give an indication about what sort of values we're looking for
rather than just to state that the value given isn't valid.  Make
BUFFER_USAGE_LIMIT uppercase in this ERROR message too.  It's already
upper case in one other error message, so make that consistent.

Reported-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20230411.102335.1643720544536884844.horikyota.ntt@gmail.com
This commit is contained in:
David Rowley 2023-04-11 19:36:34 +12:00
parent d866f0374c
commit 68a2a437f4
2 changed files with 11 additions and 28 deletions

View File

@ -195,38 +195,21 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
int result;
char *vac_buffer_size;
if (opt->arg == NULL)
{
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("buffer_usage_limit option requires a valid value"),
parser_errposition(pstate, opt->location)));
}
vac_buffer_size = defGetString(opt);
if (!parse_int(vac_buffer_size, &result, GUC_UNIT_KB, &hintmsg))
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("value: \"%s\": is invalid for buffer_usage_limit",
vac_buffer_size),
hintmsg ? errhint("%s", _(hintmsg)) : 0));
}
/*
* Check that the specified size falls within the hard upper and
* lower limits if it is not 0. We explicitly disallow -1 since
* that behavior can be obtained by not specifying
* BUFFER_USAGE_LIMIT.
* Check that the specified value is valid and the size falls
* within the hard upper and lower limits if it is not 0.
*/
if (result != 0 &&
(result < MIN_BAS_VAC_RING_SIZE_KB || result > MAX_BAS_VAC_RING_SIZE_KB))
if (!parse_int(vac_buffer_size, &result, GUC_UNIT_KB, &hintmsg) ||
(result != 0 &&
(result < MIN_BAS_VAC_RING_SIZE_KB || result > MAX_BAS_VAC_RING_SIZE_KB)))
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("buffer_usage_limit option must be 0 or between %d kB and %d kB",
MIN_BAS_VAC_RING_SIZE_KB, MAX_BAS_VAC_RING_SIZE_KB)));
errmsg("BUFFER_USAGE_LIMIT option must be 0 or between %d kB and %d kB",
MIN_BAS_VAC_RING_SIZE_KB, MAX_BAS_VAC_RING_SIZE_KB),
hintmsg ? errhint("%s", _(hintmsg)) : 0));
}
ring_size = result;

View File

@ -358,13 +358,13 @@ VACUUM (BUFFER_USAGE_LIMIT 0) vac_option_tab;
ANALYZE (BUFFER_USAGE_LIMIT 0) vac_option_tab;
-- value exceeds max size error
VACUUM (BUFFER_USAGE_LIMIT 16777220) vac_option_tab;
ERROR: buffer_usage_limit option must be 0 or between 128 kB and 16777216 kB
ERROR: BUFFER_USAGE_LIMIT option must be 0 or between 128 kB and 16777216 kB
-- value is less than min size error
VACUUM (BUFFER_USAGE_LIMIT 120) vac_option_tab;
ERROR: buffer_usage_limit option must be 0 or between 128 kB and 16777216 kB
ERROR: BUFFER_USAGE_LIMIT option must be 0 or between 128 kB and 16777216 kB
-- integer overflow error
VACUUM (BUFFER_USAGE_LIMIT 10000000000) vac_option_tab;
ERROR: value: "10000000000": is invalid for buffer_usage_limit
ERROR: BUFFER_USAGE_LIMIT option must be 0 or between 128 kB and 16777216 kB
HINT: Value exceeds integer range.
-- incompatible with VACUUM FULL error
VACUUM (BUFFER_USAGE_LIMIT '512 kB', FULL) vac_option_tab;