Cleanup VACUUM option processing error messages

The processing of the PARALLEL option for VACUUM was not quite
following what the DefElem code had intended.  defGetInt32() already has
code to handle missing parameters and returns a perfectly good error
message for when that happens.

Here we get rid of the ExecVacuum() error:

ERROR: parallel option requires a value between 0 and N

and leave defGetInt32() handle it, which will give:

ERROR:  parallel requires an integer value

defGetInt32() was already handling the non-integer parameter case, so it
may as well handle the missing parameter case too.

Additionally, parameterize the option name to make translator work easier,
and also use errhint_internal() rather than errhint() for the
BUFFER_USAGE_LIMIT option since there isn't any work for a translator to
do for "%s".

Author: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAApHDvovH14tNWB+WvP6TSbfi7-=TysQ9h5tQ5AgavwyWRWKHA@mail.gmail.com

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 733ef40ae7c524126dd9bdab2534ba812b8b23b9..e2f181eed7b8246a19d7ca6ccfeb77a3529f88d1 100644 (file)
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -220,9 +220,10 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
            {
                ereport(ERROR,
                        (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                        errmsg("BUFFER_USAGE_LIMIT option must be 0 or between %d kB and %d kB",
+                        errmsg("%s option must be 0 or between %d kB and %d kB",
+                               "BUFFER_USAGE_LIMIT",
                                MIN_BAS_VAC_RING_SIZE_KB, MAX_BAS_VAC_RING_SIZE_KB),
-                        hintmsg ? errhint("%s", _(hintmsg)) : 0));
+                        hintmsg ? errhint_internal("%s", _(hintmsg)) : 0));
            }
 
            ring_size = result;
@@ -266,35 +267,24 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
            params.truncate = get_vacoptval_from_boolean(opt);
        else if (strcmp(opt->defname, "parallel") == 0)
        {
-           if (opt->arg == NULL)
-           {
+           int         nworkers = defGetInt32(opt);
+
+           if (nworkers < 0 || nworkers > MAX_PARALLEL_WORKER_LIMIT)
                ereport(ERROR,
                        (errcode(ERRCODE_SYNTAX_ERROR),
-                        errmsg("parallel option requires a value between 0 and %d",
+                        errmsg("%s option must be between 0 and %d",
+                               "PARALLEL",
                                MAX_PARALLEL_WORKER_LIMIT),
                         parser_errposition(pstate, opt->location)));
-           }
-           else
-           {
-               int         nworkers;
-
-               nworkers = defGetInt32(opt);
-               if (nworkers < 0 || nworkers > MAX_PARALLEL_WORKER_LIMIT)
-                   ereport(ERROR,
-                           (errcode(ERRCODE_SYNTAX_ERROR),
-                            errmsg("parallel workers for vacuum must be between 0 and %d",
-                                   MAX_PARALLEL_WORKER_LIMIT),
-                            parser_errposition(pstate, opt->location)));
 
-               /*
-                * Disable parallel vacuum, if user has specified parallel
-                * degree as zero.
-                */
-               if (nworkers == 0)
-                   params.nworkers = -1;
-               else
-                   params.nworkers = nworkers;
-           }
+           /*
+            * Disable parallel vacuum, if user has specified parallel degree
+            * as zero.
+            */
+           if (nworkers == 0)
+               params.nworkers = -1;
+           else
+               params.nworkers = nworkers;
        }
        else if (strcmp(opt->defname, "skip_database_stats") == 0)
            skip_database_stats = defGetBoolean(opt);

diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index 85c783e2e56ce40cae05f5b86aeb39c9a4bfad35..d4696bc33255711acdf5988f3e46a828392bb123 100644 (file)
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -161,16 +161,14 @@ VACUUM (PARALLEL 2) pvactst;
 UPDATE pvactst SET i = i WHERE i < 1000;
 VACUUM (PARALLEL 0) pvactst; -- disable parallel vacuum
 VACUUM (PARALLEL -1) pvactst; -- error
-ERROR:  parallel workers for vacuum must be between 0 and 1024
+ERROR:  PARALLEL option must be between 0 and 1024
 LINE 1: VACUUM (PARALLEL -1) pvactst;
                 ^
 VACUUM (PARALLEL 2, INDEX_CLEANUP FALSE) pvactst;
 VACUUM (PARALLEL 2, FULL TRUE) pvactst; -- error, cannot use both PARALLEL and FULL
 ERROR:  VACUUM FULL cannot be performed in parallel
 VACUUM (PARALLEL) pvactst; -- error, cannot use PARALLEL option without parallel degree
-ERROR:  parallel option requires a value between 0 and 1024
-LINE 1: VACUUM (PARALLEL) pvactst;
-                ^
+ERROR:  parallel requires an integer value
 -- Test parallel vacuum using the minimum maintenance_work_mem with and without
 -- dead tuples.
 SET maintenance_work_mem TO 64;