Revise parse tree representation for VACUUM and ANALYZE.

Like commit f41551f61f9cf4eedd5b7173f985a3bdb4d9858c, this aims
to make it easier to add non-Boolean options to VACUUM (or, in
this case, to ANALYZE).  Instead of building up a bitmap of
options directly in the parser, build up a list of DefElem
objects and let ExecVacuum() sort it out; right now, we make
no use of the fact that a DefElem can carry an associated value,
but it will be easy to make that change in the future.

Masahiko Sawada

Discussion: http://postgr.es/m/CAD21AoATE4sn0jFFH3NcfUZXkU2BMbjBWB_kDj-XWYA-LXDcQA@mail.gmail.com

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index be03185a7f7d7d4d813ce18d0efe764cb7d1b856..f0afeaf40c27eeca9d6f0d1b5ab08528c84ecffb 100644 (file)
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -83,20 +83,55 @@ static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params);
  * happen in vacuum().
  */
 void
-ExecVacuum(VacuumStmt *vacstmt, bool isTopLevel)
+ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 {
    VacuumParams params;
+   ListCell    *lc;
+
+   params.options = vacstmt->is_vacuumcmd ? VACOPT_VACUUM : VACOPT_ANALYZE;
+
+   /* Parse options list */
+   foreach(lc, vacstmt->options)
+   {
+       DefElem *opt = (DefElem *) lfirst(lc);
+
+       /* Parse common options for VACUUM and ANALYZE */
+       if (strcmp(opt->defname, "verbose") == 0)
+           params.options |= VACOPT_VERBOSE;
+       else if (strcmp(opt->defname, "skip_locked") == 0)
+           params.options |= VACOPT_SKIP_LOCKED;
+       else if (!vacstmt->is_vacuumcmd)
+           ereport(ERROR,
+                   (errcode(ERRCODE_SYNTAX_ERROR),
+                    errmsg("unrecognized ANALYZE option \"%s\"", opt->defname),
+                    parser_errposition(pstate, opt->location)));
+
+       /* Parse options available on VACUUM */
+       else if (strcmp(opt->defname, "analyze") == 0)
+               params.options |= VACOPT_ANALYZE;
+       else if (strcmp(opt->defname, "freeze") == 0)
+               params.options |= VACOPT_FREEZE;
+       else if (strcmp(opt->defname, "full") == 0)
+           params.options |= VACOPT_FULL;
+       else if (strcmp(opt->defname, "disable_page_skipping") == 0)
+           params.options |= VACOPT_DISABLE_PAGE_SKIPPING;
+       else
+           ereport(ERROR,
+                   (errcode(ERRCODE_SYNTAX_ERROR),
+                    errmsg("unrecognized VACUUM option \"%s\"", opt->defname),
+                    parser_errposition(pstate, opt->location)));
+   }
 
    /* sanity checks on options */
-   Assert(vacstmt->options & (VACOPT_VACUUM | VACOPT_ANALYZE));
-   Assert((vacstmt->options & VACOPT_VACUUM) ||
-          !(vacstmt->options & (VACOPT_FULL | VACOPT_FREEZE)));
-   Assert(!(vacstmt->options & VACOPT_SKIPTOAST));
+   Assert(params.options & (VACOPT_VACUUM | VACOPT_ANALYZE));
+   Assert((params.options & VACOPT_VACUUM) ||
+          !(params.options & (VACOPT_FULL | VACOPT_FREEZE)));
+   Assert(!(params.options & VACOPT_SKIPTOAST));
 
    /*
     * Make sure VACOPT_ANALYZE is specified if any column lists are present.
     */
-   if (!(vacstmt->options & VACOPT_ANALYZE))
+   if (!(params.options & VACOPT_ANALYZE))
    {
        ListCell   *lc;
 
@@ -111,14 +146,11 @@ ExecVacuum(VacuumStmt *vacstmt, bool isTopLevel)
        }
    }
 
-   /* copy options from parse tree */
-   params.options = vacstmt->options;
-
    /*
     * All freeze ages are zero if the FREEZE option is given; otherwise pass
     * them as -1 which means to use the default values.
     */
-   if (vacstmt->options & VACOPT_FREEZE)
+   if (params.options & VACOPT_FREEZE)
    {
        params.freeze_min_age = 0;
        params.freeze_table_age = 0;

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index e23e68fdb332caa2b48c430a4242e871446a3f85..e814939a254ff48d83ffaf3b23f851f44cad7254 100644 (file)
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -306,8 +306,9 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
                create_extension_opt_item alter_extension_opt_item
 
 %type <ival>   opt_lock lock_type cast_context
-%type <ival>   vacuum_option_list vacuum_option_elem
-               analyze_option_list analyze_option_elem
+%type <str>        vac_analyze_option_name
+%type <defelt> vac_analyze_option_elem
+%type <list>   vac_analyze_option_list
 %type <boolean>    opt_or_replace
                opt_grant_grant_option opt_grant_admin_option
                opt_nowait opt_if_exists opt_with_data
@@ -10460,85 +10461,62 @@ cluster_index_specification:
 VacuumStmt: VACUUM opt_full opt_freeze opt_verbose opt_analyze opt_vacuum_relation_list
                {
                    VacuumStmt *n = makeNode(VacuumStmt);
-                   n->options = VACOPT_VACUUM;
+                   n->options = NIL;
                    if ($2)
-                       n->options |= VACOPT_FULL;
+                       n->options = lappend(n->options,
+                                            makeDefElem("full", NULL, @2));
                    if ($3)
-                       n->options |= VACOPT_FREEZE;
+                       n->options = lappend(n->options,
+                                            makeDefElem("freeze", NULL, @3));
                    if ($4)
-                       n->options |= VACOPT_VERBOSE;
+                       n->options = lappend(n->options,
+                                            makeDefElem("verbose", NULL, @4));
                    if ($5)
-                       n->options |= VACOPT_ANALYZE;
+                       n->options = lappend(n->options,
+                                            makeDefElem("analyze", NULL, @5));
                    n->rels = $6;
+                   n->is_vacuumcmd = true;
                    $$ = (Node *)n;
                }
-           | VACUUM '(' vacuum_option_list ')' opt_vacuum_relation_list
+           | VACUUM '(' vac_analyze_option_list ')' opt_vacuum_relation_list
                {
                    VacuumStmt *n = makeNode(VacuumStmt);
-                   n->options = VACOPT_VACUUM | $3;
+                   n->options = $3;
                    n->rels = $5;
+                   n->is_vacuumcmd = true;
                    $$ = (Node *) n;
                }
        ;
 
-vacuum_option_list:
-           vacuum_option_elem                              { $$ = $1; }
-           | vacuum_option_list ',' vacuum_option_elem     { $$ = $1 | $3; }
-       ;
-
-vacuum_option_elem:
-           analyze_keyword     { $$ = VACOPT_ANALYZE; }
-           | VERBOSE           { $$ = VACOPT_VERBOSE; }
-           | FREEZE            { $$ = VACOPT_FREEZE; }
-           | FULL              { $$ = VACOPT_FULL; }
-           | IDENT
-               {
-                   if (strcmp($1, "disable_page_skipping") == 0)
-                       $$ = VACOPT_DISABLE_PAGE_SKIPPING;
-                   else if (strcmp($1, "skip_locked") == 0)
-                       $$ = VACOPT_SKIP_LOCKED;
-                   else
-                       ereport(ERROR,
-                               (errcode(ERRCODE_SYNTAX_ERROR),
-                            errmsg("unrecognized VACUUM option \"%s\"", $1),
-                                    parser_errposition(@1)));
-               }
-       ;
-
 AnalyzeStmt: analyze_keyword opt_verbose opt_vacuum_relation_list
                {
                    VacuumStmt *n = makeNode(VacuumStmt);
-                   n->options = VACOPT_ANALYZE;
+                   n->options = NIL;
                    if ($2)
-                       n->options |= VACOPT_VERBOSE;
+                       n->options = lappend(n->options,
+                                            makeDefElem("verbose", NULL, @2));
                    n->rels = $3;
+                   n->is_vacuumcmd = false;
                    $$ = (Node *)n;
                }
-           | analyze_keyword '(' analyze_option_list ')' opt_vacuum_relation_list
+           | analyze_keyword '(' vac_analyze_option_list ')' opt_vacuum_relation_list
                {
                    VacuumStmt *n = makeNode(VacuumStmt);
-                   n->options = VACOPT_ANALYZE | $3;
+                   n->options = $3;
                    n->rels = $5;
+                   n->is_vacuumcmd = false;
                    $$ = (Node *) n;
                }
        ;
 
-analyze_option_list:
-           analyze_option_elem                             { $$ = $1; }
-           | analyze_option_list ',' analyze_option_elem   { $$ = $1 | $3; }
-       ;
-
-analyze_option_elem:
-           VERBOSE             { $$ = VACOPT_VERBOSE; }
-           | IDENT
+vac_analyze_option_list:
+           vac_analyze_option_elem
                {
-                   if (strcmp($1, "skip_locked") == 0)
-                       $$ = VACOPT_SKIP_LOCKED;
-                   else
-                       ereport(ERROR,
-                               (errcode(ERRCODE_SYNTAX_ERROR),
-                                errmsg("unrecognized ANALYZE option \"%s\"", $1),
-                                    parser_errposition(@1)));
+                   $$ = list_make1($1);
+               }
+           | vac_analyze_option_list ',' vac_analyze_option_elem
+               {
+                   $$ = lappend($1, $3);
                }
        ;
 
@@ -10547,6 +10525,18 @@ analyze_keyword:
            | ANALYSE /* British */                 {}
        ;
 
+vac_analyze_option_elem:
+           vac_analyze_option_name
+               {
+                   $$ = makeDefElem($1, NULL, @1);
+               }
+       ;
+
+vac_analyze_option_name:
+           NonReservedWord                         { $$ = $1; }
+           | analyze_keyword                       { $$ = "analyze"; }
+       ;
+
 opt_analyze:
            analyze_keyword                         { $$ = true; }
            | /*EMPTY*/                             { $$ = false; }

diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 6ec795f1b4608cf73db7a469d449dd1ac22bcc68..bdfaa506e7e25be40975114d3b2f11fb83cc374d 100644 (file)
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -664,10 +664,10 @@ standard_ProcessUtility(PlannedStmt *pstmt,
                VacuumStmt *stmt = (VacuumStmt *) parsetree;
 
                /* we choose to allow this during "read only" transactions */
-               PreventCommandDuringRecovery((stmt->options & VACOPT_VACUUM) ?
+               PreventCommandDuringRecovery(stmt->is_vacuumcmd ?
                                             "VACUUM" : "ANALYZE");
                /* forbidden in parallel mode due to CommandIsReadOnly */
-               ExecVacuum(stmt, isTopLevel);
+               ExecVacuum(pstate, stmt, isTopLevel);
            }
            break;
 
@@ -2570,7 +2570,7 @@ CreateCommandTag(Node *parsetree)
            break;
 
        case T_VacuumStmt:
-           if (((VacuumStmt *) parsetree)->options & VACOPT_VACUUM)
+           if (((VacuumStmt *) parsetree)->is_vacuumcmd)
                tag = "VACUUM";
            else
                tag = "ANALYZE";

diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index c0df9c9054b08bc472ad9f0b4b5f7633702f937d..77086f3e9130ef41624ac34208df7ef94598b25f 100644 (file)
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -136,8 +136,23 @@ typedef struct VacAttrStats
    int         rowstride;
 } VacAttrStats;
 
+typedef enum VacuumOption
+{
+   VACOPT_VACUUM = 1 << 0,     /* do VACUUM */
+   VACOPT_ANALYZE = 1 << 1,    /* do ANALYZE */
+   VACOPT_VERBOSE = 1 << 2,    /* print progress info */
+   VACOPT_FREEZE = 1 << 3,     /* FREEZE option */
+   VACOPT_FULL = 1 << 4,       /* FULL (non-concurrent) vacuum */
+   VACOPT_SKIP_LOCKED = 1 << 5,    /* skip if cannot get lock */
+   VACOPT_SKIPTOAST = 1 << 6,  /* don't process the TOAST table, if any */
+   VACOPT_DISABLE_PAGE_SKIPPING = 1 << 7   /* don't skip any pages */
+} VacuumOption;
+
 /*
  * Parameters customizing behavior of VACUUM and ANALYZE.
+ *
+ * Note that at least one of VACOPT_VACUUM and VACOPT_ANALYZE must be set
+ * in options.
  */
 typedef struct VacuumParams
 {
@@ -163,7 +178,7 @@ extern int  vacuum_multixact_freeze_table_age;
 
 
 /* in commands/vacuum.c */
-extern void ExecVacuum(VacuumStmt *vacstmt, bool isTopLevel);
+extern void ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel);
 extern void vacuum(List *relations, VacuumParams *params,
       BufferAccessStrategy bstrategy, bool isTopLevel);
 extern void vac_open_indexes(Relation relation, LOCKMODE lockmode,

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index fe35783359e4dd031278307a908f893e56a891ef..fcfba4be4c84a0194dfa6eb83a1e6d323aad53cf 100644 (file)
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3151,21 +3151,16 @@ typedef struct ClusterStmt
  *     Vacuum and Analyze Statements
  *
  * Even though these are nominally two statements, it's convenient to use
- * just one node type for both.  Note that at least one of VACOPT_VACUUM
- * and VACOPT_ANALYZE must be set in options.
+ * just one node type for both.
  * ----------------------
  */
-typedef enum VacuumOption
+typedef struct VacuumStmt
 {
-   VACOPT_VACUUM = 1 << 0,     /* do VACUUM */
-   VACOPT_ANALYZE = 1 << 1,    /* do ANALYZE */
-   VACOPT_VERBOSE = 1 << 2,    /* print progress info */
-   VACOPT_FREEZE = 1 << 3,     /* FREEZE option */
-   VACOPT_FULL = 1 << 4,       /* FULL (non-concurrent) vacuum */
-   VACOPT_SKIP_LOCKED = 1 << 5,    /* skip if cannot get lock */
-   VACOPT_SKIPTOAST = 1 << 6,  /* don't process the TOAST table, if any */
-   VACOPT_DISABLE_PAGE_SKIPPING = 1 << 7   /* don't skip any pages */
-} VacuumOption;
+   NodeTag     type;
+   List        *options;       /* list of DefElem nodes */
+   List       *rels;           /* list of VacuumRelation, or NIL for all */
+   bool        is_vacuumcmd;   /* true for VACUUM, false for ANALYZE */
+} VacuumStmt;
 
 /*
  * Info about a single target table of VACUUM/ANALYZE.
@@ -3182,13 +3177,6 @@ typedef struct VacuumRelation
    List       *va_cols;        /* list of column names, or NIL for all */
 } VacuumRelation;
 
-typedef struct VacuumStmt
-{
-   NodeTag     type;
-   int         options;        /* OR of VacuumOption flags */
-   List       *rels;           /* list of VacuumRelation, or NIL for all */
-} VacuumStmt;
-
 /* ----------------------
  *     Explain Statement
  *

diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index fa9d663abdaf7de2d488ddbebbb6838ec2ab6b52..07d0703115845ce4bf4229541c28b4d2e677b9ed 100644 (file)
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -116,8 +116,12 @@ ERROR:  column "does_not_exist" of relation "vacparted" does not exist
 ANALYZE (VERBOSE) does_not_exist;
 ERROR:  relation "does_not_exist" does not exist
 ANALYZE (nonexistent-arg) does_not_exist;
-ERROR:  unrecognized ANALYZE option "nonexistent"
+ERROR:  syntax error at or near "-"
 LINE 1: ANALYZE (nonexistent-arg) does_not_exist;
+                            ^
+ANALYZE (nonexistentarg) does_not_exit;
+ERROR:  unrecognized ANALYZE option "nonexistentarg"
+LINE 1: ANALYZE (nonexistentarg) does_not_exit;
                  ^
 -- ensure argument order independence, and that SKIP_LOCKED on non-existing
 -- relation still errors out.

diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql
index 9defa0d8b2cb33be2c074327f5c15b973508dc97..81f382267964bbc10f1e1d96195a400ab3524547 100644 (file)
--- a/src/test/regress/sql/vacuum.sql
+++ b/src/test/regress/sql/vacuum.sql
@@ -92,6 +92,7 @@ ANALYZE vactst (i), vacparted (does_not_exist);
 -- parenthesized syntax for ANALYZE
 ANALYZE (VERBOSE) does_not_exist;
 ANALYZE (nonexistent-arg) does_not_exist;
+ANALYZE (nonexistentarg) does_not_exit;
 
 -- ensure argument order independence, and that SKIP_LOCKED on non-existing
 -- relation still errors out.