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.