Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1m6VDQ-0007tC-2z for pgsql-hackers@arkaria.postgresql.org; Thu, 22 Jul 2021 09:42:04 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1m6VDP-0007Y7-3M for pgsql-hackers@arkaria.postgresql.org; Thu, 22 Jul 2021 09:42:03 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1m6VDO-0007Xp-G6 for pgsql-hackers@lists.postgresql.org; Thu, 22 Jul 2021 09:42:02 +0000 Received: from mail-io1-xd2f.google.com ([2607:f8b0:4864:20::d2f]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1m6VDK-0008Pk-8P for pgsql-hackers@postgresql.org; Thu, 22 Jul 2021 09:42:01 +0000 Received: by mail-io1-xd2f.google.com with SMTP id v26so5548090iom.11 for ; Thu, 22 Jul 2021 02:41:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=telsasoft-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=98Fwm0n891aaocXEZukpUVcuQINEUwCiqp5c2Sf7RA4=; b=HOlr+si/H06xlQOcsYP9qA11g2k03HK3KIMTtm8RkVsFUUlDLSf5Ws/GCzYhJ1pbj8 lCTzEwXujfphl0zbQyIwRK+1vOrcBeKqvVmMYnjUPA2opTJQatTSdKE+tTLF16ujb9tJ rVzBdtRSxtqjYfZWGkQxq8geIBal0jF1kAwO3RBCm2QtDQHaITMHZ1Xh0f+5w2zIYdlL 3sIDx2sNliJapOPkNT4Xfrvc759NOqRcod5dH5OqGz0IOCzKCv856mvf3ksvx/EPSrIF SRbFexZ2xwyJCDObhEIWyW5xrWAAda+7lx1diqB8eakvC7ruGsLJ4+7gqoT0Rd5mDcb7 ACxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=98Fwm0n891aaocXEZukpUVcuQINEUwCiqp5c2Sf7RA4=; b=DNwgFNA5uszuK9sYumIBrGFyLNHSxyxQj8nmeq4Cau/A3A4E4/g6dRcJtukpyvAIC5 OZN2JG99n6431BUv+N4JJCdF3K2SiqSArILddH8BWNj5tNirzlnOPSVv+ZPFInUqNf3g r8HY0lKQBR3R3v8pv2QXOoeB2D7psV2Eotso8wMBIibkzNDh/US3MGi9UfzRZ5bjRwhE MQZq8fr02Gs5RH8KJvFeszlU4o4vdCJMbBW6x9YAp+a2bPwjsC1wre+XQaSFsHhn8GxV DbzKM51CDuaqhD9jyHkFJHxN0vUwJih85n+wq2I7HuzfbHAjUXW+bcdXpH3AFcRX+8lK RURg== X-Gm-Message-State: AOAM530D7nOjajM/B8lzPPe+eNv2/x7L8Zf+aiVYzrK2i3Z2rwTyDVy9 98XEqQNDCutXf4t4KVvMHIsqCQ== X-Google-Smtp-Source: ABdhPJzkvdoCm5xOYAm6f9bWNdB0EEpSOIVgh01WMpwj5EJUYTk5WGRRJETrvfM1pzllVPPSLJfY3A== X-Received: by 2002:a05:6638:410e:: with SMTP id ay14mr4637008jab.2.1626946917417; Thu, 22 Jul 2021 02:41:57 -0700 (PDT) Received: from pryzbyj.telsasoft (charmander.telsasoft.com. [50.244.222.1]) by smtp.gmail.com with ESMTPSA id n14sm63449ilm.11.2021.07.22.02.41.56 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 22 Jul 2021 02:41:56 -0700 (PDT) Received: by pryzbyj.telsasoft (Postfix, from userid 1000) id 378A3800A39; Thu, 22 Jul 2021 04:41:55 -0500 (CDT) Date: Thu, 22 Jul 2021 04:41:54 -0500 From: Justin Pryzby To: vignesh C Cc: Jeff Davis , Michael Paquier , pgsql-hackers@postgresql.org, Jacob Champion , Heikki Linnakangas , Ashwin Agrawal , Ashutosh Sharma , Andres Freund , Robert Haas , Amit Kapila , Alvaro Herrera , Dilip Kumar Subject: Re: alter table set TABLE ACCESS METHOD Message-ID: <20210722094154.GD19620@telsasoft.com> References: <1033d6b4b5b42a014cc3f617265ae6e61cb5781b.camel@j-davis.com> <20210506221943.GS27406@telsasoft.com> <3bee08c2ca0fab15d14899afcb19490f29e0ec6f.camel@j-davis.com> <803b32e45baf0b05e5f3fc144383db3a18081693.camel@j-davis.com> <27c94b85d966351e2715cdbf79cb4c658b3cfaa7.camel@j-davis.com> <20210716034923.GL20208@telsasoft.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="K8nIJk4ghYZn606h" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --K8nIJk4ghYZn606h Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Jul 22, 2021 at 10:37:12AM +0530, vignesh C wrote: > On Fri, Jul 16, 2021 at 9:19 AM Justin Pryzby wrote: > > > > rebased. > > > > Also, there were two redundant checks for multiple SET ACCESS METHOD commands. > > But one of them wasn't hit if the ALTER was setting the current AM due to the > > no-op test. > > > > I think it's better to fail in every case, and not just sometimes (especially > > if we were to use ERRCODE_SYNTAX_ERROR). > > > > I included my 2ndary patch allowing to set the AM of partitioned table, same as > > for a tablespace. > > One of the tests is failing, please post an updated patch for this: > create_am.out 2021-07-22 10:34:56.234654166 +0530 It looks like one hunk was missing/uncommitted from the 0002 patch.. -- Justin --K8nIJk4ghYZn606h Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-ALTER-TABLE-.-SET-ACCESS-METHOD.patch" From 2e6748cadc56fad2a29a5cec895eb6796e890198 Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Wed, 5 May 2021 14:28:59 -0700 Subject: [PATCH 1/2] 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 | 68 +++++++++++++++++++++++-- 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, 175 insertions(+), 18 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 1c7f48938b..bf90040aa2 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 ] name CLUSTER ON index_name SET WITHOUT CLUSTER SET WITHOUT OIDS + SET ACCESS METHOD new_access_method SET TABLESPACE new_tablespace SET { LOGGED | UNLOGGED } SET ( storage_parameter [= value] [, ... ] ) @@ -692,6 +693,16 @@ WITH ( MODULUS numeric_literal, REM + + SET ACCESS METHOD + + + This form changes the access method of the table by rewriting it. See + for more information. + + + + SET TABLESPACE @@ -1228,6 +1239,15 @@ WITH ( MODULUS numeric_literal, REM + + new_access_method + + + The name of the access method to which the table will be converted. + + + + new_tablespace diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 2912b4c257..45bf1276a5 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -641,6 +641,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; @@ -661,6 +662,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); @@ -682,16 +684,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]; @@ -750,7 +752,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, @@ -1100,6 +1102,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; @@ -1135,6 +1141,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 25bbd8a5c1..9493b227b4 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 79abce8004..ca30399501 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 */ @@ -538,6 +539,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; @@ -4622,6 +4625,24 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE); pass = AT_PASS_DROP; break; + case AT_SetAccessMethod: /* SET ACCESS METHOD */ + ATSimplePermissions(cmd->subtype, 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 (OidIsValid(tab->newAccessMethod)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot have multiple SET ACCESS METHOD subcommands"))); + + ATPrepSetAccessMethod(tab, rel, cmd->name); + pass = AT_PASS_MISC; /* does not matter; no work in Phase 2 */ + break; case AT_SetTableSpace: /* SET TABLESPACE */ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX); @@ -4997,6 +5018,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 */ /* @@ -5324,7 +5348,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 @@ -5338,6 +5362,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; @@ -5373,11 +5398,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) @@ -5417,8 +5451,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 @@ -5933,6 +5967,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; @@ -6003,6 +6039,8 @@ alter_table_type_to_string(AlterTableType cmdtype) return "CLUSTER ON"; case AT_DropCluster: return "SET WITHOUT CLUSTER"; + case AT_SetAccessMethod: + return "SET ACCESS METHOD"; case AT_SetLogged: return "SET LOGGED"; case AT_SetUnLogged: @@ -13609,6 +13647,28 @@ 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 */ + 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 10da5c5c51..39a2849eba 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 SET ACCESS METHOD */ + | SET ACCESS METHOD name + { + AlterTableCmd *n = makeNode(AlterTableCmd); + n->subtype = AT_SetAccessMethod; + n->name = $4; + $$ = (Node *)n; + } /* ALTER TABLE SET TABLESPACE */ | SET TABLESPACE name { diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 8a44575b26..46e22411f8 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2142,13 +2142,15 @@ psql_completion(const char *text, int start, int end) } /* If we have ALTER TABLE 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 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 SET WITHOUT provide CLUSTER or OIDS */ diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h index c30ca01726..d400b6e937 100644 --- a/src/include/commands/cluster.h +++ b/src/include/commands/cluster.h @@ -36,8 +36,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 c11bf2d781..e765e67fd1 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 def9651b34..55d388485d 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 0dfb26c301..22d6a06910 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 have multiple SET ACCESS METHOD subcommands +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 9a359466ce..d0d39d71ec 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.0 --K8nIJk4ghYZn606h Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0002-Allow-specifying-access-method-of-partitioned-tables.patch" From b5542eb5747cf05c599b4d08e0e06d737f54d861 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 7 Mar 2021 00:11:38 -0600 Subject: [PATCH 2/2] Allow specifying access method of partitioned tables.. ..to be inherited by partitions See also: ca4103025dfe26eaaf6a500dec9170fbb176eebc 8586bf7ed8889f39a59dd99b292014b73be85342 ebfe2dbd6b624e2a428e14b7ee9322cc096f63f7 - prevent DROP AM --- src/backend/catalog/heap.c | 3 +- src/backend/commands/tablecmds.c | 76 ++++++++++++++++++------- src/backend/utils/cache/lsyscache.c | 22 +++++++ src/backend/utils/cache/relcache.c | 4 +- src/include/utils/lsyscache.h | 1 + src/test/regress/expected/create_am.out | 21 +++---- src/test/regress/sql/create_am.sql | 6 +- 7 files changed, 98 insertions(+), 35 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 83746d3fd9..57b6146b4f 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -1464,12 +1464,13 @@ heap_create_with_catalog(const char *relname, /* * Make a dependency link to force the relation to be deleted if its - * access method is. Do this only for relation and materialized views. + * access method is. Do this only for relevant types. * * No need to add an explicit dependency for the toast table, as the * main table depends on it. */ if (relkind == RELKIND_RELATION || + relkind == RELKIND_PARTITIONED_TABLE || relkind == RELKIND_MATVIEW) { ObjectAddressSet(referenced, AccessMethodRelationId, accessmtd); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index ca30399501..ed058b8b99 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -540,6 +540,7 @@ 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 void ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethod); static bool ATPrepChangePersistence(Relation rel, bool toLogged); static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, const char *tablespacename, LOCKMODE lockmode); @@ -646,7 +647,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, Oid ofTypeId; ObjectAddress address; LOCKMODE parentLockmode; - const char *accessMethod = NULL; Oid accessMethodId = InvalidOid; /* @@ -908,23 +908,23 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, * a type of relation that needs one, use the default. */ if (stmt->accessMethod != NULL) + accessMethodId = get_table_am_oid(stmt->accessMethod, false); + else if (stmt->partbound && relkind == RELKIND_RELATION) { - accessMethod = stmt->accessMethod; - - if (partitioned) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("specifying a table access method is not supported on a partitioned table"))); - + /* + * For partitioned tables, when no access method is specified, we + * default to the parent table's AM. + */ + Assert(list_length(inheritOids) == 1); + accessMethodId = get_rel_relam(linitial_oid(inheritOids)); + if (!OidIsValid(accessMethodId)) + accessMethodId = get_table_am_oid(default_table_access_method, false); } else if (relkind == RELKIND_RELATION || relkind == RELKIND_TOASTVALUE || + relkind == RELKIND_PARTITIONED_TABLE || relkind == RELKIND_MATVIEW) - accessMethod = default_table_access_method; - - /* look up the access method, verify it is for a table */ - if (accessMethod != NULL) - accessMethodId = get_table_am_oid(accessMethod, false); + accessMethodId = get_table_am_oid(default_table_access_method, false); /* * Create the relation. Inherited defaults and constraints are passed in @@ -4628,12 +4628,6 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, case AT_SetAccessMethod: /* SET ACCESS METHOD */ ATSimplePermissions(cmd->subtype, 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 (OidIsValid(tab->newAccessMethod)) ereport(ERROR, @@ -5020,6 +5014,8 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, break; case AT_SetAccessMethod: /* SET ACCESS METHOD */ /* handled specially in Phase 3 */ + if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + ATExecSetAccessMethodNoStorage(rel, tab->newAccessMethod); break; case AT_SetTableSpace: /* SET TABLESPACE */ @@ -13993,6 +13989,48 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) list_free(reltoastidxids); } +/* + * Special handling of ALTER TABLE SET ACCESS METHOD for relations with no + * storage that have an interest in preserving AM. + * + * Since these have no storage, setting the access method is a catalog only + * operation. + */ +static void +ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethod) +{ + Relation pg_class; + Oid relid; + HeapTuple tuple; + + /* + * Shouldn't be called on relations having storage; these are processed in + * phase 3. + */ + Assert(!RELKIND_HAS_STORAGE(rel->rd_rel->relkind)); + + relid = RelationGetRelid(rel); + + /* Pull the record for this relation and update it */ + pg_class = table_open(RelationRelationId, RowExclusiveLock); + + tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid)); + + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for relation %u", relid); + + ((Form_pg_class) GETSTRUCT(tuple))->relam = newAccessMethod; + CatalogTupleUpdate(pg_class, &tuple->t_self, tuple); + + InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), 0); + + heap_freetuple(tuple); + table_close(pg_class, RowExclusiveLock); + + /* Make sure the relam change is visible */ + CommandCounterIncrement(); +} + /* * Special handling of ALTER TABLE SET TABLESPACE for relations with no * storage that have an interest in preserving tablespace. diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c index 6bba5f8ec4..703d8d06be 100644 --- a/src/backend/utils/cache/lsyscache.c +++ b/src/backend/utils/cache/lsyscache.c @@ -2062,6 +2062,28 @@ get_rel_persistence(Oid relid) return result; } +/* + * get_rel_relam + * + * Returns the relam associated with a given relation. + */ +Oid +get_rel_relam(Oid relid) +{ + HeapTuple tp; + Form_pg_class reltup; + Oid result; + + tp = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); + if (!HeapTupleIsValid(tp)) + elog(ERROR, "cache lookup failed for relation %u", relid); + reltup = (Form_pg_class) GETSTRUCT(tp); + result = reltup->relam; + ReleaseSysCache(tp); + + return result; +} + /* ---------- TRANSFORM CACHE ---------- */ diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 13d9994af3..35c72fa74d 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -1187,9 +1187,11 @@ RelationBuildDesc(Oid targetRelId, bool insertIt) case RELKIND_VIEW: case RELKIND_COMPOSITE_TYPE: case RELKIND_FOREIGN_TABLE: - case RELKIND_PARTITIONED_TABLE: Assert(relation->rd_rel->relam == InvalidOid); break; + case RELKIND_PARTITIONED_TABLE: + /* Do nothing: it's a catalog settings for partitions to inherit */ + break; } /* extract reloptions if any */ diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h index 77871aaefc..10145570fd 100644 --- a/src/include/utils/lsyscache.h +++ b/src/include/utils/lsyscache.h @@ -139,6 +139,7 @@ extern char get_rel_relkind(Oid relid); extern bool get_rel_relispartition(Oid relid); extern Oid get_rel_tablespace(Oid relid); extern char get_rel_persistence(Oid relid); +extern Oid get_rel_relam(Oid relid); extern Oid get_transform_fromsql(Oid typid, Oid langid, List *trftypes); extern Oid get_transform_tosql(Oid typid, Oid langid, List *trftypes); extern bool get_typisdefined(Oid typid); diff --git a/src/test/regress/expected/create_am.out b/src/test/regress/expected/create_am.out index 22d6a06910..19e4c19bfb 100644 --- a/src/test/regress/expected/create_am.out +++ b/src/test/regress/expected/create_am.out @@ -176,11 +176,9 @@ SELECT f1 FROM tableam_tblmv_heap2 ORDER BY f1; 1 (1 row) --- CREATE TABLE .. PARTITION BY doesn't not support USING -CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a) USING heap2; -ERROR: specifying a table access method is not supported on a partitioned table -CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a); +-- CREATE TABLE .. PARTITION BY supports USING -- new partitions will inherit from the current default, rather the partition root +CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a) USING heap2; SET default_table_access_method = 'heap'; CREATE TABLE tableam_parted_a_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('a'); SET default_table_access_method = 'heap2'; @@ -205,14 +203,17 @@ WHERE pa.oid = pc.relam ORDER BY 3, 1, 2; relkind | amname | relname ---------+--------+---------------------------------- + r | heap2 | tableam_parted_a_heap2 r | heap2 | tableam_parted_b_heap2 r | heap2 | tableam_parted_d_heap2 + p | heap2 | tableam_parted_heap2 r | heap2 | tableam_tbl_heap2 r | heap2 | tableam_tblas_heap2 m | heap2 | tableam_tblmv_heap2 + t | heap2 | toast for tableam_parted_a_heap2 t | heap2 | toast for tableam_parted_b_heap2 t | heap2 | toast for tableam_parted_d_heap2 -(7 rows) +(10 rows) -- Show dependencies onto AM - there shouldn't be any for toast SELECT pg_describe_object(classid,objid,objsubid) AS obj @@ -226,9 +227,11 @@ ORDER BY classid, objid, objsubid; table tableam_tbl_heap2 table tableam_tblas_heap2 materialized view tableam_tblmv_heap2 + table tableam_parted_heap2 + table tableam_parted_a_heap2 table tableam_parted_b_heap2 table tableam_parted_d_heap2 -(5 rows) +(7 rows) -- ALTER TABLE SET ACCESS METHOD CREATE TABLE heaptable USING heap AS @@ -262,7 +265,6 @@ 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; @@ -300,7 +302,7 @@ ORDER BY 3, 1, 2; f | | tableam_fdw_heapx r | heap2 | tableam_parted_1_heapx r | heap | tableam_parted_2_heapx - p | | tableam_parted_heapx + p | heap2 | tableam_parted_heapx S | | tableam_seq_heapx r | heap2 | tableam_tbl_heapx r | heap2 | tableam_tblas_heapx @@ -329,7 +331,6 @@ ERROR: cannot drop access method heap2 because other objects depend on it DETAIL: table tableam_tbl_heap2 depends on access method heap2 table tableam_tblas_heap2 depends on access method heap2 materialized view tableam_tblmv_heap2 depends on access method heap2 -table tableam_parted_b_heap2 depends on access method heap2 -table tableam_parted_d_heap2 depends on access method heap2 +table tableam_parted_heap2 depends on access method heap2 HINT: Use DROP ... CASCADE to drop the dependent objects too. -- we intentionally leave the objects created above alive, to verify pg_dump support diff --git a/src/test/regress/sql/create_am.sql b/src/test/regress/sql/create_am.sql index d0d39d71ec..28984cab1f 100644 --- a/src/test/regress/sql/create_am.sql +++ b/src/test/regress/sql/create_am.sql @@ -124,11 +124,9 @@ CREATE SEQUENCE tableam_seq_heap2 USING heap2; CREATE MATERIALIZED VIEW tableam_tblmv_heap2 USING heap2 AS SELECT * FROM tableam_tbl_heap2; SELECT f1 FROM tableam_tblmv_heap2 ORDER BY f1; --- CREATE TABLE .. PARTITION BY doesn't not support USING -CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a) USING heap2; - -CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a); +-- CREATE TABLE .. PARTITION BY supports USING -- new partitions will inherit from the current default, rather the partition root +CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a) USING heap2; SET default_table_access_method = 'heap'; CREATE TABLE tableam_parted_a_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('a'); SET default_table_access_method = 'heap2'; -- 2.17.0 --K8nIJk4ghYZn606h--