From 68a2a437f46716efd54d177edf9dbcc9916c903b Mon Sep 17 00:00:00 2001 From: David Rowley Date: Tue, 11 Apr 2023 19:36:34 +1200 Subject: [PATCH] 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 --- src/backend/commands/vacuum.c | 33 +++++++--------------------- src/test/regress/expected/vacuum.out | 6 ++--- 2 files changed, 11 insertions(+), 28 deletions(-) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 873209d946a..a843f9ad926 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -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; diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out index 044b761024b..41e020cf20c 100644 --- a/src/test/regress/expected/vacuum.out +++ b/src/test/regress/expected/vacuum.out @@ -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;