On Wed, 2021-06-09 at 13:47 +0900, Michael Paquier wrote:
> There is a mix of upper and lower-case characters here. It could be
> more consistent. It seems to me that this test should actually check
> that pg_class.relam reflects the new value.
Done. I also added a (negative) test for changing the AM of a
partitioned table, which wasn't caught before.
> + errmsg("cannot have multiple SET ACCESS METHOD
> subcommands")));
> Worth adding a test?
Done.
> Nit: the name of the variable looks inconsistent with this comment.
> The original is weird as well.
Tried to improve wording.
> I am wondering if it would be a good idea to set the new tablespace
> and new access method fields to InvalidOid within
> ATGetQueueEntry. We
> do that for the persistence. Not critical at all, still..
Done.
> + pass = AT_PASS_MISC;
> Maybe add a comment here?
Done. In that case, it doesn't matter because there's no work to be
done in Phase 2.
Regards,
Jeff Davis
From 051d067e07eaec29d221641da3bf28d0dd0731c8 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
Date: Wed, 5 May 2021 14:28:59 -0700
Subject: [PATCH] ALTER TABLE ... SET ACCESS METHOD
Adds support for changing the access method of a table with a
rewrite.
Author: Justin Pryzby, Jeff Davis
---
doc/src/sgml/ref/alter_table.sgml | 20 +++++++
src/backend/commands/cluster.c | 21 +++++---
src/backend/commands/matview.c | 5 +-
src/backend/commands/tablecmds.c | 71 +++++++++++++++++++++++--
src/backend/parser/gram.y | 8 +++
src/bin/psql/tab-complete.c | 10 ++--
src/include/commands/cluster.h | 4 +-
src/include/commands/event_trigger.h | 1 +
src/include/nodes/parsenodes.h | 1 +
src/test/regress/expected/create_am.out | 34 ++++++++++++
src/test/regress/sql/create_am.sql | 21 ++++++++
11 files changed, 178 insertions(+), 18 deletions(-)
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 939d3fe2739..5ac13a5c0f6 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -75,6 +75,7 @@ ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
CLUSTER ON <replaceable class="parameter">index_name</replaceable>
SET WITHOUT CLUSTER
SET WITHOUT OIDS
+ SET ACCESS METHOD <replaceable class="parameter">new_access_method</replaceable>
SET TABLESPACE <replaceable class="parameter">new_tablespace</replaceable>
SET { LOGGED | UNLOGGED }
SET ( <replaceable class="parameter">storage_parameter</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] )
@@ -693,6 +694,16 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>SET ACCESS METHOD</literal></term>
+ <listitem>
+ <para>
+ This form changes the access method of the table by rewriting it. See
+ <xref linkend="tableam"/> for more information.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>SET TABLESPACE</literal></term>
<listitem>
@@ -1229,6 +1240,15 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><replaceable class="parameter">new_access_method</replaceable></term>
+ <listitem>
+ <para>
+ The name of the access method to which the table will be converted.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><replaceable class="parameter">new_tablespace</replaceable></term>
<listitem>
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 6487a9e3fcb..b3d8b6deb03 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -576,6 +576,7 @@ static void
rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
{
Oid tableOid = RelationGetRelid(OldHeap);
+ Oid accessMethod = OldHeap->rd_rel->relam;
Oid tableSpace = OldHeap->rd_rel->reltablespace;
Oid OIDNewHeap;
char relpersistence;
@@ -597,6 +598,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
/* Create the transient table that will receive the re-ordered data */
OIDNewHeap = make_new_heap(tableOid, tableSpace,
+ accessMethod,
relpersistence,
AccessExclusiveLock);
@@ -618,16 +620,16 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
/*
* Create the transient table that will be filled with new data during
* CLUSTER, ALTER TABLE, and similar operations. The transient table
- * duplicates the logical structure of the OldHeap, but is placed in
- * NewTableSpace which might be different from OldHeap's. Also, it's built
- * with the specified persistence, which might differ from the original's.
+ * duplicates the logical structure of the OldHeap; but will have the
+ * specified physical storage properties NewTableSpace, NewAccessMethod, and
+ * relpersistence.
*
* After this, the caller should load the new heap with transferred/modified
* data, then call finish_heap_swap to complete the operation.
*/
Oid
-make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, char relpersistence,
- LOCKMODE lockmode)
+make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod,
+ char relpersistence, LOCKMODE lockmode)
{
TupleDesc OldHeapDesc;
char NewHeapName[NAMEDATALEN];
@@ -686,7 +688,7 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, char relpersistence,
InvalidOid,
InvalidOid,
OldHeap->rd_rel->relowner,
- OldHeap->rd_rel->relam,
+ NewAccessMethod,
OldHeapDesc,
NIL,
RELKIND_RELATION,
@@ -1036,6 +1038,10 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
relform1->reltablespace = relform2->reltablespace;
relform2->reltablespace = swaptemp;
+ swaptemp = relform1->relam;
+ relform1->relam = relform2->relam;
+ relform2->relam = swaptemp;
+
swptmpchr = relform1->relpersistence;
relform1->relpersistence = relform2->relpersistence;
relform2->relpersistence = swptmpchr;
@@ -1071,6 +1077,9 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
if (relform1->relpersistence != relform2->relpersistence)
elog(ERROR, "cannot change persistence of mapped relation \"%s\"",
NameStr(relform1->relname));
+ if (relform1->relam != relform2->relam)
+ elog(ERROR, "cannot change access method of mapped relation \"%s\"",
+ NameStr(relform1->relname));
if (!swap_toast_by_content &&
(relform1->reltoastrelid || relform2->reltoastrelid))
elog(ERROR, "cannot swap toast by links for mapped relation \"%s\"",
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 25bbd8a5c14..9493b227b4b 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -298,8 +298,9 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
* it against access by any other process until commit (by which time it
* will be gone).
*/
- OIDNewHeap = make_new_heap(matviewOid, tableSpace, relpersistence,
- ExclusiveLock);
+ OIDNewHeap = make_new_heap(matviewOid, tableSpace,
+ matviewRel->rd_rel->relam,
+ relpersistence, ExclusiveLock);
LockRelationOid(OIDNewHeap, AccessExclusiveLock);
dest = CreateTransientRelDestReceiver(OIDNewHeap);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 028e8ac46b3..e0721566562 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -176,6 +176,7 @@ typedef struct AlteredTableInfo
List *afterStmts; /* List of utility command parsetrees */
bool verify_new_notnull; /* T if we should recheck NOT NULL */
int rewrite; /* Reason for forced rewrite, if any */
+ Oid newAccessMethod; /* new access method; 0 means no change */
Oid newTableSpace; /* new tablespace; 0 means no change */
bool chgPersistence; /* T if SET LOGGED/UNLOGGED is used */
char newrelpersistence; /* if above is true */
@@ -539,6 +540,7 @@ static void change_owner_recurse_to_sequences(Oid relationOid,
static ObjectAddress ATExecClusterOn(Relation rel, const char *indexName,
LOCKMODE lockmode);
static void ATExecDropCluster(Relation rel, LOCKMODE lockmode);
+static void ATPrepSetAccessMethod(AlteredTableInfo *tab, Relation rel, const char *amname);
static bool ATPrepChangePersistence(Relation rel, bool toLogged);
static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
const char *tablespacename, LOCKMODE lockmode);
@@ -4096,6 +4098,7 @@ AlterTableGetLockLevel(List *cmds)
*/
case AT_AddColumn: /* may rewrite heap, in some cases and visible
* to SELECT */
+ case AT_SetAccessMethod: /* must rewrite heap */
case AT_SetTableSpace: /* must rewrite heap */
case AT_AlterColumnType: /* must rewrite heap */
cmd_lockmode = AccessExclusiveLock;
@@ -4623,6 +4626,24 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
pass = AT_PASS_DROP;
break;
+ case AT_SetAccessMethod: /* SET ACCESS METHOD */
+ ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW);
+
+ /* partitioned tables don't have an access method */
+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot change access method of a partitioned table")));
+
+ /* check if another access method change was already requested */
+ if (tab->newAccessMethod)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot change access method setting twice")));
+
+ ATPrepSetAccessMethod(tab, rel, cmd->name);
+ pass = AT_PASS_MISC; /* does not matter; no work in Phase 2 */
+ break;
case AT_SetTableSpace: /* SET TABLESPACE */
ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX |
ATT_PARTITIONED_INDEX);
@@ -4998,6 +5019,9 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
case AT_DropOids: /* SET WITHOUT OIDS */
/* nothing to do here, oid columns don't exist anymore */
break;
+ case AT_SetAccessMethod: /* SET ACCESS METHOD */
+ /* handled specially in Phase 3 */
+ break;
case AT_SetTableSpace: /* SET TABLESPACE */
/*
@@ -5325,7 +5349,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
/*
* We only need to rewrite the table if at least one column needs to
- * be recomputed, or we are changing its persistence.
+ * be recomputed, or we are changing its persistence or access method.
*
* There are two reasons for requiring a rewrite when changing
* persistence: on one hand, we need to ensure that the buffers
@@ -5339,6 +5363,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
/* Build a temporary relation and copy data */
Relation OldHeap;
Oid OIDNewHeap;
+ Oid NewAccessMethod;
Oid NewTableSpace;
char persistence;
@@ -5374,11 +5399,20 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
* Select destination tablespace (same as original unless user
* requested a change)
*/
- if (tab->newTableSpace)
+ if (OidIsValid(tab->newTableSpace))
NewTableSpace = tab->newTableSpace;
else
NewTableSpace = OldHeap->rd_rel->reltablespace;
+ /*
+ * Select destination access method (same as original unless user
+ * requested a change)
+ */
+ if (OidIsValid(tab->newAccessMethod))
+ NewAccessMethod = tab->newAccessMethod;
+ else
+ NewAccessMethod = OldHeap->rd_rel->relam;
+
/*
* Select persistence of transient table (same as original unless
* user requested a change)
@@ -5418,8 +5452,8 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
* persistence. That wouldn't work for pg_class, but that can't be
* unlogged anyway.
*/
- OIDNewHeap = make_new_heap(tab->relid, NewTableSpace, persistence,
- lockmode);
+ OIDNewHeap = make_new_heap(tab->relid, NewTableSpace, NewAccessMethod,
+ persistence, lockmode);
/*
* Copy the heap data into the new table with the desired
@@ -5934,6 +5968,8 @@ ATGetQueueEntry(List **wqueue, Relation rel)
tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
+ tab->newTableSpace = InvalidOid;
+ tab->newAccessMethod = InvalidOid;
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
tab->chgPersistence = false;
@@ -13520,6 +13556,33 @@ ATExecDropCluster(Relation rel, LOCKMODE lockmode)
mark_index_clustered(rel, InvalidOid, false);
}
+/*
+ * Preparation phase for SET ACCESS METHOD
+ *
+ * Check that access method exists. If it's the same as the table's current
+ * access method, it's a no-op. Otherwise, a table rewrite is necessary.
+ */
+static void
+ATPrepSetAccessMethod(AlteredTableInfo *tab, Relation rel, const char *amname)
+{
+ Oid amoid;
+
+ /* Check that the table access method exists */
+ amoid = get_table_am_oid(amname, false);
+
+ if (rel->rd_rel->relam == amoid)
+ return;
+
+ /* Save info for Phase 3 to do the real work */
+ if (OidIsValid(tab->newAccessMethod))
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("cannot have multiple SET ACCESS METHOD subcommands")));
+
+ tab->rewrite |= AT_REWRITE_ACCESS_METHOD;
+ tab->newAccessMethod = amoid;
+}
+
/*
* ALTER TABLE SET TABLESPACE
*/
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 52a254928f8..8d3e21d21fc 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2623,6 +2623,14 @@ alter_table_cmd:
n->newowner = $3;
$$ = (Node *)n;
}
+ /* ALTER TABLE <name> SET ACCESS METHOD <amname> */
+ | SET ACCESS METHOD name
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ n->subtype = AT_SetAccessMethod;
+ n->name = $4;
+ $$ = (Node *)n;
+ }
/* ALTER TABLE <name> SET TABLESPACE <tablespacename> */
| SET TABLESPACE name
{
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 32c1bdfdca7..25332d7cc77 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2137,13 +2137,15 @@ psql_completion(const char *text, int start, int end)
}
/* If we have ALTER TABLE <sth> SET, provide list of attributes and '(' */
else if (Matches("ALTER", "TABLE", MatchAny, "SET"))
- COMPLETE_WITH("(", "LOGGED", "SCHEMA", "TABLESPACE", "UNLOGGED",
- "WITH", "WITHOUT");
+ COMPLETE_WITH("(", "ACCESS METHOD", "LOGGED", "SCHEMA",
+ "TABLESPACE", "UNLOGGED", "WITH", "WITHOUT");
/*
- * If we have ALTER TABLE <sth> SET TABLESPACE provide a list of
- * tablespaces
+ * Complete with list of tablespaces (for SET TABLESPACE) or table AMs (for
+ * SET ACCESS METHOD).
*/
+ else if (Matches("ALTER", "TABLE", MatchAny, "SET", "ACCESS", "METHOD"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_table_access_methods);
else if (Matches("ALTER", "TABLE", MatchAny, "SET", "TABLESPACE"))
COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces);
/* If we have ALTER TABLE <sth> SET WITHOUT provide CLUSTER or OIDS */
diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h
index a941f2accda..b64d3bc2040 100644
--- a/src/include/commands/cluster.h
+++ b/src/include/commands/cluster.h
@@ -35,8 +35,8 @@ extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid,
bool recheck, LOCKMODE lockmode);
extern void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal);
-extern Oid make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, char relpersistence,
- LOCKMODE lockmode);
+extern Oid make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod,
+ char relpersistence, LOCKMODE lockmode);
extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
bool is_system_catalog,
bool swap_toast_by_content,
diff --git a/src/include/commands/event_trigger.h b/src/include/commands/event_trigger.h
index c11bf2d7810..e765e67fd10 100644
--- a/src/include/commands/event_trigger.h
+++ b/src/include/commands/event_trigger.h
@@ -32,6 +32,7 @@ typedef struct EventTriggerData
#define AT_REWRITE_ALTER_PERSISTENCE 0x01
#define AT_REWRITE_DEFAULT_VAL 0x02
#define AT_REWRITE_COLUMN_REWRITE 0x04
+#define AT_REWRITE_ACCESS_METHOD 0x08
/*
* EventTriggerData is the node type that is passed as fmgr "context" info
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index ef73342019f..e2ee7907f0d 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1901,6 +1901,7 @@ typedef enum AlterTableType
AT_SetLogged, /* SET LOGGED */
AT_SetUnLogged, /* SET UNLOGGED */
AT_DropOids, /* SET WITHOUT OIDS */
+ AT_SetAccessMethod, /* SET ACCESS METHOD */
AT_SetTableSpace, /* SET TABLESPACE */
AT_SetRelOptions, /* SET (...) -- AM specific parameters */
AT_ResetRelOptions, /* RESET (...) -- AM specific parameters */
diff --git a/src/test/regress/expected/create_am.out b/src/test/regress/expected/create_am.out
index 0dfb26c3014..e999b00542c 100644
--- a/src/test/regress/expected/create_am.out
+++ b/src/test/regress/expected/create_am.out
@@ -230,6 +230,40 @@ ORDER BY classid, objid, objsubid;
table tableam_parted_d_heap2
(5 rows)
+-- ALTER TABLE SET ACCESS METHOD
+CREATE TABLE heaptable USING heap AS
+ SELECT a, repeat(a::text,9999) FROM generate_series(1,9) AS a;
+SELECT amname FROM pg_class c, pg_am am
+ WHERE c.relam = am.oid AND c.oid = 'heaptable'::regclass;
+ amname
+--------
+ heap
+(1 row)
+
+ALTER TABLE heaptable SET ACCESS METHOD heap2;
+SELECT amname FROM pg_class c, pg_am am
+ WHERE c.relam = am.oid AND c.oid = 'heaptable'::regclass;
+ amname
+--------
+ heap2
+(1 row)
+
+SELECT COUNT(a), COUNT(1) FILTER(WHERE a=1) FROM heaptable;
+ count | count
+-------+-------
+ 9 | 1
+(1 row)
+
+-- negative test
+ALTER TABLE heaptable SET ACCESS METHOD heap, SET ACCESS METHOD heap2;
+ERROR: cannot change access method setting twice
+DROP TABLE heaptable;
+CREATE TABLE am_partitioned(x INT, y INT)
+ PARTITION BY hash (x);
+-- negative test
+ALTER TABLE am_partitioned SET ACCESS METHOD heap2;
+ERROR: cannot change access method of a partitioned table
+DROP TABLE am_partitioned;
-- Second, create objects in the new AM by changing the default AM
BEGIN;
SET LOCAL default_table_access_method = 'heap2';
diff --git a/src/test/regress/sql/create_am.sql b/src/test/regress/sql/create_am.sql
index 9a359466ce4..d0d39d71ecf 100644
--- a/src/test/regress/sql/create_am.sql
+++ b/src/test/regress/sql/create_am.sql
@@ -161,6 +161,27 @@ WHERE pg_depend.refclassid = 'pg_am'::regclass
AND pg_am.amname = 'heap2'
ORDER BY classid, objid, objsubid;
+-- ALTER TABLE SET ACCESS METHOD
+CREATE TABLE heaptable USING heap AS
+ SELECT a, repeat(a::text,9999) FROM generate_series(1,9) AS a;
+SELECT amname FROM pg_class c, pg_am am
+ WHERE c.relam = am.oid AND c.oid = 'heaptable'::regclass;
+ALTER TABLE heaptable SET ACCESS METHOD heap2;
+SELECT amname FROM pg_class c, pg_am am
+ WHERE c.relam = am.oid AND c.oid = 'heaptable'::regclass;
+SELECT COUNT(a), COUNT(1) FILTER(WHERE a=1) FROM heaptable;
+
+-- negative test
+ALTER TABLE heaptable SET ACCESS METHOD heap, SET ACCESS METHOD heap2;
+
+DROP TABLE heaptable;
+
+CREATE TABLE am_partitioned(x INT, y INT)
+ PARTITION BY hash (x);
+
+-- negative test
+ALTER TABLE am_partitioned SET ACCESS METHOD heap2;
+DROP TABLE am_partitioned;
-- Second, create objects in the new AM by changing the default AM
BEGIN;
--
2.17.1
