Fix parsing of REPACK options

Previously, REPACK option parsing had two bugs.

First, REPACK (CONCURRENTLY OFF) failed with:

    ERROR:  unrecognized REPACK option "concurrently"

while CONCURRENTLY ON was accepted correctly.

Second, when the same option was specified multiple times, the last value
specified was not always honored. If any occurrence set the option to ON,
the option was treated as enabled even when the final setting was OFF.

This commit fixes these issues by correctly accepting CONCURRENTLY
regardless of its value, and by making the last specified value take precedence
when an option appears multiple times.

Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/CAHGQGwGAY4kfDtC4i+hAOX-a3u0yOA6__6EDTQz-ytsDHgh-yQ@mail.gmail.com

diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index fae88d6bb83170fe5726671ffcec23e185d0d5d6..351a3cc32e8f1ca0499d44d04cb9ee32ccb5bcf3 100644 (file)
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -248,24 +248,26 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel)
        MemoryContext repack_context;
        LOCKMODE        lockmode;
        List       *rtcs;
+       bool            verbose = false;
+       bool            analyze = false;
+       bool            concurrently = false;
 
        /* Parse option list */
        foreach_node(DefElem, opt, stmt->params)
        {
                if (strcmp(opt->defname, "verbose") == 0)
-                       params.options |= defGetBoolean(opt) ? CLUOPT_VERBOSE : 0;
+                       verbose = defGetBoolean(opt);
                else if (strcmp(opt->defname, "analyze") == 0 ||
                                 strcmp(opt->defname, "analyse") == 0)
-                       params.options |= defGetBoolean(opt) ? CLUOPT_ANALYZE : 0;
-               else if (strcmp(opt->defname, "concurrently") == 0 &&
-                                defGetBoolean(opt))
+                       analyze = defGetBoolean(opt);
+               else if (strcmp(opt->defname, "concurrently") == 0)
                {
                        if (stmt->command != REPACK_COMMAND_REPACK)
                                ereport(ERROR,
                                                errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                                                errmsg("CONCURRENTLY option not supported for %s",
                                                           RepackCommandAsString(stmt->command)));
-                       params.options |= CLUOPT_CONCURRENT;
+                       concurrently = defGetBoolean(opt);
                }
                else
                        ereport(ERROR,
@@ -276,6 +278,11 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel)
                                        parser_errposition(pstate, opt->location));
        }
 
+       params.options |=
+               (verbose ? CLUOPT_VERBOSE : 0) |
+               (analyze ? CLUOPT_ANALYZE : 0) |
+               (concurrently ? CLUOPT_CONCURRENT : 0);
+
        /* Determine the lock mode to use. */
        lockmode = RepackLockLevel((params.options & CLUOPT_CONCURRENT) != 0);