From 8e9caa318c877871f2bab2f096d2406121166f3b Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Thu, 26 Mar 2020 22:08:40 -0500 Subject: [PATCH v16 6/7] fix!Parenthesized syntax: VACUUM/CLUSTER (TABLESPACE) --- doc/src/sgml/ref/cluster.sgml | 19 ++++++++- doc/src/sgml/ref/vacuum.sgml | 14 ++++++- src/backend/commands/cluster.c | 19 +++++++++ src/backend/commands/vacuum.c | 51 ++++++++++++----------- src/backend/nodes/copyfuncs.c | 1 - src/backend/nodes/equalfuncs.c | 1 - src/backend/parser/gram.y | 36 +--------------- src/include/nodes/parsenodes.h | 2 - src/test/regress/input/tablespace.source | 14 +++---- src/test/regress/output/tablespace.source | 14 +++---- 10 files changed, 90 insertions(+), 81 deletions(-) diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index a61a77a014..0e81e6189b 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -21,8 +21,14 @@ PostgreSQL documentation -CLUSTER [VERBOSE] table_name [ USING index_name ] [ TABLESPACE new_tablespace ] +CLUSTER [VERBOSE] [ ( option [, ...] ) ] table_name [ USING index_name ] CLUSTER [VERBOSE] + +where option can be one of: + + VERBOSE + TABLESPACE new_tablespace + @@ -90,6 +96,15 @@ CLUSTER [VERBOSE] + + TABLESPACE + + + Specifies that the relation will be rebuilt on a new tablespace. + + + + table_name @@ -112,7 +127,7 @@ CLUSTER [VERBOSE] new_tablespace - The tablespace where the clustered relation will be rebuilt. + The tablespace where the relation will be rebuilt. diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml index 681486bb38..b44b10c783 100644 --- a/doc/src/sgml/ref/vacuum.sgml +++ b/doc/src/sgml/ref/vacuum.sgml @@ -22,8 +22,8 @@ PostgreSQL documentation VACUUM [ ( option [, ...] ) ] [ table_and_columns [, ...] ] -VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ TABLESPACE new_tablespace ] [ table_and_columns [, ...] ] -VACUUM ( FULL [, ...] ) [ TABLESPACE new_tablespace ] [ table_and_columns [, ...] ] +VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ table_and_columns [, ...] ] +VACUUM ( FULL [, ...] ) [ table_and_columns [, ...] ] where option can be one of: @@ -36,6 +36,7 @@ VACUUM ( FULL [, ...] ) [ TABLESPACE new_tablespa INDEX_CLEANUP [ boolean ] TRUNCATE [ boolean ] PARALLEL integer + TABLESPACE new_tablespace and table_and_columns is: @@ -256,6 +257,15 @@ VACUUM ( FULL [, ...] ) [ TABLESPACE new_tablespa + + TABLESPACE + + + Specifies that the relation will be rebuilt on a new tablespace. + + + + boolean diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index ef8b1c9a82..cef4beb2c8 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -105,6 +105,9 @@ void cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) { ListCell *lc; + /* Name and Oid of tablespace to use for clustered relation. */ + char *tablespaceName = NULL; + Oid tablespaceOid = InvalidOid; /* Parse list of generic parameter not handled by the parser */ foreach(lc, stmt->params) @@ -114,6 +117,9 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) if (strcmp(opt->defname, "verbose") == 0) stmt->options |= CLUOPT_VERBOSE; // XXX: handle boolean opt: VERBOSE off + else if (strcmp(opt->defname, "tablespace") == 0) + tablespaceName = defGetString(opt); + // XXX: if (tablespaceOid == GLOBALTABLESPACE_OID) else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -122,6 +128,19 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) parser_errposition(pstate, opt->location))); } + /* Select tablespace Oid to use. */ + if (tablespaceName) + { + tablespaceOid = get_tablespace_oid(tablespaceName, false); + + /* Can't move a non-shared relation into pg_global */ + if (tablespaceOid == GLOBALTABLESPACE_OID) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move non-shared relation to tablespace \"%s\"", + tablespaceName))); + } + if (stmt->relation != NULL) { /* This is the single-relation case. */ diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 95bec8d6ba..005fb97a7b 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -109,8 +109,10 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) bool disable_page_skipping = false; bool parallel_option = false; ListCell *lc; - Oid tablespaceOid = InvalidOid; /* Oid of tablespace to use for relations - * after VACUUM FULL. */ + + /* Name and Oid of tablespace to use for relations after VACUUM FULL. */ + char *tablespacename = NULL; + Oid tablespaceOid = InvalidOid; /* Set default value */ params.index_cleanup = VACOPT_TERNARY_DEFAULT; @@ -148,6 +150,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) params.index_cleanup = get_vacopt_ternary_value(opt); else if (strcmp(opt->defname, "truncate") == 0) params.truncate = get_vacopt_ternary_value(opt); + else if (strcmp(opt->defname, "tablespace") == 0) + tablespacename = defGetString(opt); else if (strcmp(opt->defname, "parallel") == 0) { parallel_option = true; @@ -209,6 +213,27 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot specify both FULL and PARALLEL options"))); + /* Get tablespace Oid to use. */ + if (tablespacename) + { + if ((params.options & VACOPT_FULL) == 0) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("incompatible TABLESPACE option"), + errdetail("You can only use TABLESPACE with VACUUM FULL."))); + + tablespaceOid = get_tablespace_oid(tablespacename, false); + + /* Can't move a non-shared relation into pg_global */ + if (tablespaceOid == GLOBALTABLESPACE_OID) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move non-shared relation to tablespace \"%s\"", + tablespacename))); + + } + params.tablespace_oid = tablespaceOid; + /* * Make sure VACOPT_ANALYZE is specified if any column lists are present. */ @@ -246,28 +271,6 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) params.multixact_freeze_table_age = -1; } - /* Get tablespace Oid to use. */ - if (vacstmt->tablespacename) - { - if (params.options & VACOPT_FULL) - { - tablespaceOid = get_tablespace_oid(vacstmt->tablespacename, false); - - /* Can't move a non-shared relation into pg_global */ - if (tablespaceOid == GLOBALTABLESPACE_OID) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move non-shared relation to tablespace \"%s\"", - vacstmt->tablespacename))); - } - else - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("incompatible TABLESPACE option"), - errdetail("You can only use TABLESPACE with VACUUM FULL."))); - } - params.tablespace_oid = tablespaceOid; - /* user-invoked vacuum is never "for wraparound" */ params.is_wraparound = false; diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index e04ee775c2..a2aa687771 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3903,7 +3903,6 @@ _copyVacuumStmt(const VacuumStmt *from) COPY_NODE_FIELD(options); COPY_NODE_FIELD(rels); COPY_SCALAR_FIELD(is_vacuumcmd); - COPY_STRING_FIELD(tablespacename); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 9ffad8be90..3967d0ce08 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1703,7 +1703,6 @@ _equalVacuumStmt(const VacuumStmt *a, const VacuumStmt *b) COMPARE_NODE_FIELD(options); COMPARE_NODE_FIELD(rels); COMPARE_SCALAR_FIELD(is_vacuumcmd); - COMPARE_SCALAR_FIELD(tablespacename); return true; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index a91617c4e2..c152d78846 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -10626,7 +10626,7 @@ CreateConversionStmt: *****************************************************************************/ ClusterStmt: - CLUSTER opt_verbose qualified_name cluster_index_specification OptTableSpace + CLUSTER opt_verbose qualified_name cluster_index_specification { ClusterStmt *n = makeNode(ClusterStmt); n->relation = $3; @@ -10707,28 +10707,6 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose opt_analyze opt_vacuum_relati makeDefElem("analyze", NULL, @5)); n->rels = $6; n->is_vacuumcmd = true; - n->tablespacename = NULL; - $$ = (Node *)n; - } - | VACUUM opt_full opt_freeze opt_verbose opt_analyze TABLESPACE name opt_vacuum_relation_list - { - VacuumStmt *n = makeNode(VacuumStmt); - n->options = NIL; - if ($2) - n->options = lappend(n->options, - makeDefElem("full", NULL, @2)); - if ($3) - n->options = lappend(n->options, - makeDefElem("freeze", NULL, @3)); - if ($4) - n->options = lappend(n->options, - makeDefElem("verbose", NULL, @4)); - if ($5) - n->options = lappend(n->options, - makeDefElem("analyze", NULL, @5)); - n->tablespacename = $7; - n->rels = $8; - n->is_vacuumcmd = true; $$ = (Node *)n; } | VACUUM '(' vac_analyze_option_list ')' opt_vacuum_relation_list @@ -10737,16 +10715,6 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose opt_analyze opt_vacuum_relati n->options = $3; n->rels = $5; n->is_vacuumcmd = true; - n->tablespacename = NULL; - $$ = (Node *) n; - } - | VACUUM '(' vac_analyze_option_list ')' TABLESPACE name opt_vacuum_relation_list - { - VacuumStmt *n = makeNode(VacuumStmt); - n->options = $3; - n->tablespacename = $6; - n->rels = $7; - n->is_vacuumcmd = true; $$ = (Node *) n; } ; @@ -10760,7 +10728,6 @@ AnalyzeStmt: analyze_keyword opt_verbose opt_vacuum_relation_list makeDefElem("verbose", NULL, @2)); n->rels = $3; n->is_vacuumcmd = false; - n->tablespacename = NULL; $$ = (Node *)n; } | analyze_keyword '(' vac_analyze_option_list ')' opt_vacuum_relation_list @@ -10769,7 +10736,6 @@ AnalyzeStmt: analyze_keyword opt_verbose opt_vacuum_relation_list n->options = $3; n->rels = $5; n->is_vacuumcmd = false; - n->tablespacename = NULL; $$ = (Node *) n; } ; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 2236aaa2dc..21b1f51950 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3203,7 +3203,6 @@ typedef struct ClusterStmt NodeTag type; RangeVar *relation; /* relation being indexed, or NULL if all */ char *indexname; /* original index defined */ - char *tablespacename; /* tablespace name to use for clustered relation */ int options; /* OR of ClusterOption flags */ List *params; /* Params not further parsed by the grammar */ } ClusterStmt; @@ -3221,7 +3220,6 @@ typedef struct VacuumStmt List *options; /* list of DefElem nodes */ List *rels; /* list of VacuumRelation, or NIL for all */ bool is_vacuumcmd; /* true for VACUUM, false for ANALYZE */ - char *tablespacename; /* tablespace name to use for vacuumed relation */ } VacuumStmt; /* diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source index 03c5fb9e9e..6942775bfb 100644 --- a/src/test/regress/input/tablespace.source +++ b/src/test/regress/input/tablespace.source @@ -47,9 +47,9 @@ WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspa ORDER BY relname; -- check CLUSTER with TABLESPACE change -CLUSTER regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx TABLESPACE regress_tblspace; -- ok -CLUSTER pg_authid USING pg_authid_rolname_index TABLESPACE regress_tblspace; -- fail -CLUSTER regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx TABLESPACE pg_global; -- fail +CLUSTER (TABLESPACE regress_tblspace) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- ok +CLUSTER (TABLESPACE regress_tblspace) pg_authid USING pg_authid_rolname_index; -- fail +CLUSTER (TABLESPACE pg_global) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- fail -- check that all relations moved to new tablespace SELECT relname FROM pg_class @@ -57,10 +57,10 @@ WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspa ORDER BY relname; -- check VACUUM with TABLESPACE change -VACUUM (FULL, ANALYSE, FREEZE) TABLESPACE pg_default regress_tblspace_test_tbl; -- ok -VACUUM (FULL) TABLESPACE pg_default pg_authid; -- skip with warning -VACUUM (ANALYSE) TABLESPACE pg_default; -- fail -VACUUM (FULL) TABLESPACE pg_global regress_tblspace_test_tbl; -- fail +VACUUM (FULL, ANALYSE, FREEZE, TABLESPACE pg_default) regress_tblspace_test_tbl; -- ok +VACUUM (FULL, TABLESPACE pg_default) pg_authid; -- skip with warning +VACUUM (ANALYSE, TABLESPACE pg_default); -- fail +VACUUM (FULL, TABLESPACE pg_global) regress_tblspace_test_tbl; -- fail -- check that all tables moved back to pg_default SELECT relname FROM pg_class diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source index c27fa70143..2825984394 100644 --- a/src/test/regress/output/tablespace.source +++ b/src/test/regress/output/tablespace.source @@ -60,10 +60,10 @@ ORDER BY relname; (1 row) -- check CLUSTER with TABLESPACE change -CLUSTER regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx TABLESPACE regress_tblspace; -- ok -CLUSTER pg_authid USING pg_authid_rolname_index TABLESPACE regress_tblspace; -- fail +CLUSTER (TABLESPACE regress_tblspace) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- ok +CLUSTER (TABLESPACE regress_tblspace) pg_authid USING pg_authid_rolname_index; -- fail ERROR: cannot cluster a shared catalog -CLUSTER regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx TABLESPACE pg_global; -- fail +CLUSTER (TABLESPACE pg_global) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- fail ERROR: cannot move non-shared relation to tablespace "pg_global" -- check that all relations moved to new tablespace SELECT relname FROM pg_class @@ -76,14 +76,14 @@ ORDER BY relname; (2 rows) -- check VACUUM with TABLESPACE change -VACUUM (FULL, ANALYSE, FREEZE) TABLESPACE pg_default regress_tblspace_test_tbl; -- ok -VACUUM (FULL) TABLESPACE pg_default pg_authid; -- skip with warning +VACUUM (FULL, ANALYSE, FREEZE, TABLESPACE pg_default) regress_tblspace_test_tbl; -- ok +VACUUM (FULL, TABLESPACE pg_default) pg_authid; -- skip with warning WARNING: skipping tablespace change of "pg_authid" DETAIL: Cannot move system relation, only VACUUM is performed. -VACUUM (ANALYSE) TABLESPACE pg_default; -- fail +VACUUM (ANALYSE, TABLESPACE pg_default); -- fail ERROR: incompatible TABLESPACE option DETAIL: You can only use TABLESPACE with VACUUM FULL. -VACUUM (FULL) TABLESPACE pg_global regress_tblspace_test_tbl; -- fail +VACUUM (FULL, TABLESPACE pg_global) regress_tblspace_test_tbl; -- fail ERROR: cannot move non-shared relation to tablespace "pg_global" -- check that all tables moved back to pg_default SELECT relname FROM pg_class -- 2.17.0