From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Peter Eisentraut <peter@eisentraut.org>
Cc: Soumyadeep Chakraborty <soumyadeep2007@gmail.com>
Cc: Zhihong Yu <zyu@yugabyte.com>
Cc: pgsql-hackers@postgresql.org, Ashwin Agrawal <ashwinstar@gmail.com>
Cc: vanjared@vmware.com
Subject: Re: ALTER TABLE SET ACCESS METHOD on partitioned tables
Date: Thu, 7 Mar 2024 20:02:00 -0600
Message-ID: <ZepxmAvTNYfptrQi@pryzbyj2023> (raw)
In-Reply-To: <ZeWKgCtk6xiAsDsc@paquier.xyz>
References: <ZHiUKhhT71X4CjJl@paquier.xyz>
<ZLgn3Ou4YpgP8Z8l@telsasoft.com>
<d1cb16ec-9af1-4657-81f2-865480d2bd6d@eisentraut.org>
<ZdWa0sSExPfBvAyR@paquier.xyz>
<cab23590-19d4-4895-9333-1c25f4194741@eisentraut.org>
<Zd7qEUKkBVY0d2-4@paquier.xyz>
<ZeCZ89xAVFeOmrQC@pryzbyj2023>
<ZeE14kii-re1CSUk@paquier.xyz>
<ZeJCkt6UPLwQIn2S@pryzbyj2023>
<ZeWKgCtk6xiAsDsc@paquier.xyz>
On Mon, Mar 04, 2024 at 05:46:56PM +0900, Michael Paquier wrote:
> > Since InvalidOid is already taken, I guess you might need to introduce a
> > boolean flag, like set_relam, indicating that the statement has an
> > ACCESS METHOD clause.
>
> Yes, I don't see an alternative. The default needs a different field
> to be tracked down to the execution.
The data structure you used (defaultAccessMethod) allows this, which
is intended to be prohibited:
postgres=# ALTER TABLE a SET access method default, SET access method default;
ALTER TABLE
As you wrote it, you pass the "defaultAccessMethod" bool to
ATExecSetAccessMethodNoStorage(), which seems odd. Why not just pass
the target amoid as newAccessMethod ?
When I fooled with this on my side, I called it "chgAccessMethod" to
follow "chgPersistence". I think "is default" isn't the right data
structure.
Attached a relative patch with my version.
Also: I just realized that instead of adding a bool, we could test
(tab->rewrite & AT_REWRITE_ACCESS_METHOD) != 0
+-- Default and AM set in in clause are the same, relam should be set.
in in?
--
Justin
Attachments:download all (2 files, .tar.gz)0001-Allow-specifying-access-method-of-partitioned-tables.patch (text/x-diff, 29.4K) download
From 8aca5e443ebb41b7de1ba18b519a33a97a41a897 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 4 Mar 2024 17:42:04 +0900
Subject: [PATCH 1/2] Allow specifying access method of partitioned tables
..to be inherited by partitions
See also:
ca4103025dfe26eaaf6a500dec9170fbb176eebc
8586bf7ed8889f39a59dd99b292014b73be85342
ebfe2dbd6b624e2a428e14b7ee9322cc096f63f7 - prevent DROP AM
Authors: Justin Pryzby, Soumyadeep Chakraborty
---
doc/src/sgml/catalogs.sgml | 5 +-
doc/src/sgml/ref/alter_table.sgml | 8 ++
doc/src/sgml/ref/create_table.sgml | 4 +
src/backend/commands/tablecmds.c | 183 +++++++++++++++++++++---
src/backend/utils/cache/lsyscache.c | 22 +++
src/backend/utils/cache/relcache.c | 7 +
src/bin/pg_dump/pg_dump.c | 3 +-
src/bin/pg_dump/t/002_pg_dump.pl | 35 +++++
src/include/catalog/pg_class.h | 4 +-
src/include/utils/lsyscache.h | 1 +
src/test/regress/expected/create_am.out | 158 +++++++++++++++++++-
src/test/regress/sql/create_am.sql | 91 +++++++++++-
12 files changed, 487 insertions(+), 34 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgmlindex 0ae97d1adaa..e4fcacb610a 100644--- a/doc/src/sgml/catalogs.sgml+++ b/doc/src/sgml/catalogs.sgml@@ -1989,8 +1989,9 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
</para>
<para>
If this is a table or an index, the access method used (heap,
- B-tree, hash, etc.); otherwise zero (zero occurs for sequences,- as well as relations without storage, such as views)+ B-tree, hash, etc.); otherwise zero. Zero occurs for sequences,+ as well as relations without storage, such as views. Partitioned+ tables may use a non-zero value.
</para></entry>
</row>
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgmlindex 96e3d776051..779c8b31226 100644--- a/doc/src/sgml/ref/alter_table.sgml+++ b/doc/src/sgml/ref/alter_table.sgml@@ -737,6 +737,14 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
<literal>DEFAULT</literal> changes the access method of the table
to <xref linkend="guc-default-table-access-method"/>.
</para>
+ <para>+ When applied to a partitioned table, there is no data to rewrite, but any+ partitions created afterwards will use that access method unless+ overridden by a <literal>USING</literal> clause. Using+ <varname>DEFAULT</varname> switches the partitioned table to rely on+ the value of <varname>default_table_access_method</varname> set+ when new partitions are created, instead.+ </para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgmlindex 4cbaaccaf7c..b79081a5ec1 100644--- a/doc/src/sgml/ref/create_table.sgml+++ b/doc/src/sgml/ref/create_table.sgml@@ -1330,6 +1330,10 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
method is chosen for the new table. See <xref
linkend="guc-default-table-access-method"/> for more information.
</para>
+ <para>+ When creating a partition, the table access method is the access method+ of its partitioned table, if set.+ </para>
</listitem>
</varlistentry>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 7014be80396..cf595329b6c 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -184,6 +184,7 @@ typedef struct AlteredTableInfo
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 */
+ bool defaultAccessMethod; /* true if SET ACCESS METHOD DEFAULT */
Oid newTableSpace; /* new tablespace; 0 means no change */
bool chgPersistence; /* T if SET LOGGED/UNLOGGED is used */
char newrelpersistence; /* if above is true */
@@ -589,6 +590,8 @@ 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,+ bool defaultAccessMethod);
static bool ATPrepChangePersistence(Relation rel, bool toLogged);
static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
const char *tablespacename, LOCKMODE lockmode);
@@ -703,7 +706,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
Oid ofTypeId;
ObjectAddress address;
LOCKMODE parentLockmode;
- const char *accessMethod = NULL;
Oid accessMethodId = InvalidOid;
/*
@@ -956,20 +958,25 @@ 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)
{
- 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 partitions, if no access method is specified, use the AM of the+ * parent table.+ */+ Assert(list_length(inheritOids) == 1);+ accessMethodId = get_rel_relam(linitial_oid(inheritOids));
}
- else if (RELKIND_HAS_TABLE_AM(relkind))- accessMethod = default_table_access_method;+ else+ accessMethodId = InvalidOid;- /* look up the access method, verify it is for a table */- if (accessMethod != NULL)- accessMethodId = get_table_am_oid(accessMethod, false);+ /*+ * Still nothing? Use the default. Partitioned tables default to+ * InvalidOid without an access method specified.+ */+ if (RELKIND_HAS_TABLE_AM(relkind) && !OidIsValid(accessMethodId))+ accessMethodId = get_table_am_oid(default_table_access_method, false);
/*
* Create the relation. Inherited defaults and constraints are passed in
@@ -5045,12 +5052,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,
@@ -5407,6 +5408,14 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
break;
case AT_SetAccessMethod: /* SET ACCESS METHOD */
/* handled specially in Phase 3 */
++ /*+ * Only do this for partitioned tables, for which this is just a+ * catalog change. Tables with storage are handled by Phase 3.+ */+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)+ ATExecSetAccessMethodNoStorage(rel, tab->newAccessMethod,+ tab->defaultAccessMethod);
break;
case AT_SetTableSpace: /* SET TABLESPACE */
@@ -6399,6 +6408,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newAccessMethod = InvalidOid;
+ tab->defaultAccessMethod = false;
tab->newTableSpace = InvalidOid;
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
tab->chgPersistence = false;
@@ -15201,8 +15211,14 @@ ATExecDropCluster(Relation rel, LOCKMODE lockmode)
* Preparation phase for SET ACCESS METHOD
*
* Check that access method exists. If it is the same as the table's current
- * access method, it is a no-op. Otherwise, a table rewrite is necessary.+ * access method, it is a no-op. Otherwise, a table rewrite is necessary for+ * relations with storage.
* If amname is NULL, select default_table_access_method as access method.
+ *+ * Partitioned tables may use InvalidOid to assign the default AM to partitions+ * when created. This operation is a no-op if a DEFAULT was given and the+ * partitioned table uses InvalidOid as relam, or if the relam and the access+ * method match.
*/
static void
ATPrepSetAccessMethod(AlteredTableInfo *tab, Relation rel, const char *amname)
@@ -15213,12 +15229,35 @@ ATPrepSetAccessMethod(AlteredTableInfo *tab, Relation rel, const char *amname)
amoid = get_table_am_oid(amname ? amname : default_table_access_method,
false);
- if (rel->rd_rel->relam == amoid)+ if (RELKIND_HAS_STORAGE(rel->rd_rel->relkind) &&+ rel->rd_rel->relam == amoid)
return;
+ /* Partitioned tables are handled here */+ if (!RELKIND_HAS_STORAGE(rel->rd_rel->relkind))+ {+ if (amname == NULL)+ {+ /* DEFAULT implied, leave if relam is already InvalidOid */+ if (!OidIsValid(rel->rd_rel->relam))+ return;+ }+ else+ {+ /*+ * Access method set by query, leave if only this matches with its+ * relam.+ */+ if (OidIsValid(rel->rd_rel->relam) &&+ amoid == rel->rd_rel->relam)+ return;+ }+ }+
/* Save info for Phase 3 to do the real work */
tab->rewrite |= AT_REWRITE_ACCESS_METHOD;
tab->newAccessMethod = amoid;
+ tab->defaultAccessMethod = (amname == NULL);
}
/*
@@ -15543,6 +15582,110 @@ 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. defaultAccessMethod tracks if a default value is wanted for+ * the access method.+ */+static void+ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethodId,+ bool defaultAccessMethod)+{+ Relation pg_class;+ Oid oldAccessMethodId;+ HeapTuple tuple;+ Form_pg_class rd_rel;+ Oid reloid = RelationGetRelid(rel);++ if (!OidIsValid(newAccessMethodId))+ {+ /* if the access method is unchanged, leave */+ return;+ }++ /*+ * Shouldn't be called on relations having storage; these are processed in+ * phase 3.+ */+ Assert(!RELKIND_HAS_STORAGE(rel->rd_rel->relkind));++ /* Get a modifiable copy of the relation's pg_class row. */+ pg_class = table_open(RelationRelationId, RowExclusiveLock);++ tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));+ if (!HeapTupleIsValid(tuple))+ elog(ERROR, "cache lookup failed for relation %u", reloid);+ rd_rel = (Form_pg_class) GETSTRUCT(tuple);++ /* Update the pg_class row. */+ oldAccessMethodId = rd_rel->relam;+ rd_rel->relam = defaultAccessMethod ? InvalidOid : newAccessMethodId;++ /* Leave if no update required */+ if (rd_rel->relam == oldAccessMethodId)+ {+ heap_freetuple(tuple);+ table_close(pg_class, RowExclusiveLock);+ return;+ }++ CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);++ /*+ * Update the dependency on the new access method. No dependency is added+ * if the new access method is InvalidOid (default case). Be very careful+ * that this has to compare the previous value stored in pg_class with the+ * new one.+ */+ if (!OidIsValid(oldAccessMethodId) && OidIsValid(rd_rel->relam))+ {+ ObjectAddress relobj,+ referenced;++ /*+ * New access method is defined and there was no dependency+ * previously, so record a new one.+ */+ ObjectAddressSet(relobj, RelationRelationId, reloid);+ ObjectAddressSet(referenced, AccessMethodRelationId, rd_rel->relam);+ recordDependencyOn(&relobj, &referenced, DEPENDENCY_NORMAL);+ }+ else if (OidIsValid(oldAccessMethodId) &&+ !OidIsValid(rd_rel->relam))+ {+ /*+ * There was an access method defined, and no new one, so just remove+ * the existing dependency.+ */+ deleteDependencyRecordsForClass(RelationRelationId, reloid,+ AccessMethodRelationId,+ DEPENDENCY_NORMAL);+ }+ else+ {+ Assert(OidIsValid(oldAccessMethodId) &&+ OidIsValid(rd_rel->relam));++ /* Both are valid, so update the dependency */+ changeDependencyFor(RelationRelationId, reloid,+ AccessMethodRelationId,+ oldAccessMethodId, rd_rel->relam);+ }++ CommandCounterIncrement();++ 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.cindex 6418d1c6eba..26368ffcc97 100644--- a/src/backend/utils/cache/lsyscache.c+++ b/src/backend/utils/cache/lsyscache.c@@ -2069,6 +2069,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.cindex 4d339ee7950..18c6044238c 100644--- a/src/backend/utils/cache/relcache.c+++ b/src/backend/utils/cache/relcache.c@@ -1208,6 +1208,13 @@ retry:
else if (RELKIND_HAS_TABLE_AM(relation->rd_rel->relkind) ||
relation->rd_rel->relkind == RELKIND_SEQUENCE)
RelationInitTableAccessMethod(relation);
+ else if (relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)+ {+ /*+ * Do nothing: access methods are a setting that partitions can+ * inherit.+ */+ }
else
Assert(relation->rd_rel->relam == InvalidOid);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.cindex 10cbf02bebd..ec406bff297 100644--- a/src/bin/pg_dump/pg_dump.c+++ b/src/bin/pg_dump/pg_dump.c@@ -16610,7 +16610,8 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
if (RELKIND_HAS_TABLESPACE(tbinfo->relkind))
tablespace = tbinfo->reltablespace;
- if (RELKIND_HAS_TABLE_AM(tbinfo->relkind))+ if (RELKIND_HAS_TABLE_AM(tbinfo->relkind) ||+ tbinfo->relkind == RELKIND_PARTITIONED_TABLE)
tableam = tbinfo->amname;
ArchiveEntry(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId,
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.plindex 00b5092713d..dfee1982f41 100644--- a/src/bin/pg_dump/t/002_pg_dump.pl+++ b/src/bin/pg_dump/t/002_pg_dump.pl@@ -4587,6 +4587,41 @@ my %tests = (
no_table_access_method => 1,
only_dump_measurement => 1,
},
+ },++ # CREATE TABLE with partitioned table and various AMs. One+ # partition uses the same default as the parent, and a second+ # uses its own AM.+ 'CREATE TABLE regress_pg_dump_table_part' => {+ create_order => 19,+ create_sql => '+ CREATE TABLE dump_test.regress_pg_dump_table_am_parent (id int) PARTITION BY LIST (id);+ ALTER TABLE dump_test.regress_pg_dump_table_am_parent SET ACCESS METHOD regress_table_am;+ CREATE TABLE dump_test.regress_pg_dump_table_am_child_1+ PARTITION OF dump_test.regress_pg_dump_table_am_parent FOR VALUES IN (1) USING heap;+ CREATE TABLE dump_test.regress_pg_dump_table_am_child_2+ PARTITION OF dump_test.regress_pg_dump_table_am_parent FOR VALUES IN (2);',+ regexp => qr/^+ \QSET default_table_access_method = regress_table_am;\E+ (\n(?!SET[^;]+;)[^\n]*)*+ \n\QCREATE TABLE dump_test.regress_pg_dump_table_am_parent (\E+ (.*\n)*+ \QSET default_table_access_method = heap;\E+ (\n(?!SET[^;]+;)[^\n]*)*+ \n\QCREATE TABLE dump_test.regress_pg_dump_table_am_child_1 (\E+ (.*\n)*+ \QSET default_table_access_method = regress_table_am;\E+ (\n(?!SET[^;]+;)[^\n]*)*+ \n\QCREATE TABLE dump_test.regress_pg_dump_table_am_child_2 (\E+ (.*\n)*/xm,+ like => {+ %full_runs, %dump_test_schema_runs, section_pre_data => 1,+ },+ unlike => {+ exclude_dump_test_schema => 1,+ no_table_access_method => 1,+ only_dump_measurement => 1,+ },
});
#########################################
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.hindex 3b7533e7bb3..b505ed1becc 100644--- a/src/include/catalog/pg_class.h+++ b/src/include/catalog/pg_class.h@@ -219,7 +219,9 @@ MAKE_SYSCACHE(RELNAMENSP, pg_class_relname_nsp_index, 128);
/*
* Relation kinds with a table access method (rd_tableam). Although sequences
* use the heap table AM, they are enough of a special case in most uses that
- * they are not included here.+ * they are not included here. Likewise, partitioned tables can have an access+ * method defined so that their children can inherit it; however, this is+ * handled specially outside of this macro.
*/
#define RELKIND_HAS_TABLE_AM(relkind) \
((relkind) == RELKIND_RELATION || \
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.hindex e4a200b00ec..35a8dec2b9f 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.outindex 8d73e213563..438f5a72193 100644--- a/src/test/regress/expected/create_am.out+++ b/src/test/regress/expected/create_am.out@@ -176,9 +176,16 @@ SELECT f1 FROM tableam_tblmv_heap2 ORDER BY f1;
1
(1 row)
--- CREATE TABLE .. PARTITION BY doesn't not support USING+-- CREATE TABLE .. PARTITION BY supports 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+SELECT a.amname FROM pg_class c, pg_am a+ WHERE c.relname = 'tableam_parted_heap2' AND a.oid = c.relam;+ amname +--------+ heap2+(1 row)++DROP TABLE tableam_parted_heap2;
CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a);
-- new partitions will inherit from the current default, rather the partition root
SET default_table_access_method = 'heap';
@@ -336,12 +343,151 @@ ALTER MATERIALIZED VIEW heapmv SET ACCESS METHOD heap, SET ACCESS METHOD heap2;
ERROR: cannot have multiple SET ACCESS METHOD subcommands
DROP MATERIALIZED VIEW heapmv;
DROP TABLE heaptable;
--- No support for partitioned tables.-CREATE TABLE am_partitioned(x INT, y INT)- PARTITION BY hash (x);+-- Partition hierarchies with access methods+BEGIN;+SET LOCAL default_table_access_method = 'heap';+CREATE TABLE am_partitioned(x INT, y INT) PARTITION BY hash (x);+-- pg_class.relam is 0, no dependency recorded between the AM and the+-- partitioned table.+SELECT relam FROM pg_class WHERE relname = 'am_partitioned';+ relam +-------+ 0+(1 row)++SELECT pg_describe_object(classid, objid, objsubid) AS obj,+ pg_describe_object(refclassid, refobjid, refobjsubid) as refobj+ FROM pg_depend, pg_am+ WHERE pg_depend.refclassid = 'pg_am'::regclass+ AND pg_am.oid = pg_depend.refobjid+ AND pg_depend.objid = 'am_partitioned'::regclass;+ obj | refobj +-----+--------+(0 rows)++-- New default is set, with dependency added.+ALTER TABLE am_partitioned SET ACCESS METHOD heap2;+SELECT a.amname FROM pg_class c, pg_am a+ WHERE c.relname = 'am_partitioned' AND a.oid = c.relam;+ amname +--------+ heap2+(1 row)++SELECT pg_describe_object(classid, objid, objsubid) AS obj,+ pg_describe_object(refclassid, refobjid, refobjsubid) as refobj+ FROM pg_depend, pg_am+ WHERE pg_depend.refclassid = 'pg_am'::regclass+ AND pg_am.oid = pg_depend.refobjid+ AND pg_depend.objid = 'am_partitioned'::regclass;+ obj | refobj +----------------------+---------------------+ table am_partitioned | access method heap2+(1 row)++-- Default is set, with dependency updated.+SET LOCAL default_table_access_method = 'heap2';+ALTER TABLE am_partitioned SET ACCESS METHOD heap;+SELECT a.amname FROM pg_class c, pg_am a+ WHERE c.relname = 'am_partitioned' AND a.oid = c.relam;+ amname +--------+ heap+(1 row)++-- Dependency pinned, hence removed.+SELECT pg_describe_object(classid, objid, objsubid) AS obj,+ pg_describe_object(refclassid, refobjid, refobjsubid) as refobj+ FROM pg_depend, pg_am+ WHERE pg_depend.refclassid = 'pg_am'::regclass+ AND pg_am.oid = pg_depend.refobjid+ AND pg_depend.objid = 'am_partitioned'::regclass;+ obj | refobj +-----+--------+(0 rows)++-- Default and AM set in in clause are the same, relam should be set.+SET LOCAL default_table_access_method = 'heap2';+ALTER TABLE am_partitioned SET ACCESS METHOD heap2;+SELECT a.amname FROM pg_class c, pg_am a+ WHERE c.relname = 'am_partitioned' AND a.oid = c.relam;+ amname +--------+ heap2+(1 row)++-- Reset to default+ALTER TABLE am_partitioned SET ACCESS METHOD DEFAULT;+SELECT relam FROM pg_class WHERE relname = 'am_partitioned';+ relam +-------+ 0+(1 row)++-- Upon ALTER TABLE SET ACCESS METHOD on a partitioned table, new partitions+-- will inherit the AM set. Existing partitioned are unchanged.+SELECT relam FROM pg_class WHERE relname = 'am_partitioned';+ relam +-------+ 0+(1 row)++SET LOCAL default_table_access_method = 'heap';+CREATE TABLE am_partitioned_0 PARTITION OF am_partitioned+ FOR VALUES WITH (MODULUS 10, REMAINDER 0);+SET LOCAL default_table_access_method = 'heap2';+CREATE TABLE am_partitioned_1 PARTITION OF am_partitioned+ FOR VALUES WITH (MODULUS 10, REMAINDER 1);+SET LOCAL default_table_access_method = 'heap';
ALTER TABLE am_partitioned SET ACCESS METHOD heap2;
-ERROR: cannot change access method of a partitioned table+CREATE TABLE am_partitioned_2 PARTITION OF am_partitioned+ FOR VALUES WITH (MODULUS 10, REMAINDER 2);+ALTER TABLE am_partitioned SET ACCESS METHOD DEFAULT;+SELECT relam FROM pg_class WHERE relname = 'am_partitioned';+ relam +-------+ 0+(1 row)++CREATE TABLE am_partitioned_3 PARTITION OF am_partitioned+ FOR VALUES WITH (MODULUS 10, REMAINDER 3);+-- Partitioned table with relam at 0+ALTER TABLE am_partitioned SET ACCESS METHOD DEFAULT;+CREATE TABLE am_partitioned_5p PARTITION OF am_partitioned+ FOR VALUES WITH (MODULUS 10, REMAINDER 5) PARTITION BY hash(y);+-- Partitions of this partitioned table inherit default AM at creation+-- time.+CREATE TABLE am_partitioned_5p1 PARTITION OF am_partitioned_5p+ FOR VALUES WITH (MODULUS 10, REMAINDER 1);+-- Partitioned table with relam set.+ALTER TABLE am_partitioned SET ACCESS METHOD heap2;+CREATE TABLE am_partitioned_6p PARTITION OF am_partitioned+ FOR VALUES WITH (MODULUS 10, REMAINDER 6) PARTITION BY hash(y);+-- Partitions of this partitioned table inherit its AM.+CREATE TABLE am_partitioned_6p1 PARTITION OF am_partitioned_6p+ FOR VALUES WITH (MODULUS 10, REMAINDER 1);+SELECT c.relname, a.amname FROM pg_class c, pg_am a+ WHERE c.relam = a.oid AND+ c.relname LIKE 'am_partitioned%'+UNION ALL+SELECT c.relname, 'default' FROM pg_class c+ WHERE c.relam = 0+ AND c.relname LIKE 'am_partitioned%' ORDER BY 1;+ relname | amname +--------------------+---------+ am_partitioned | heap2+ am_partitioned_0 | heap+ am_partitioned_1 | heap2+ am_partitioned_2 | heap2+ am_partitioned_3 | heap+ am_partitioned_5p | default+ am_partitioned_5p1 | heap+ am_partitioned_6p | heap2+ am_partitioned_6p1 | heap2+(9 rows)+
DROP TABLE am_partitioned;
+COMMIT;
-- 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.sqlindex 606ee4cb241..907211653d3 100644--- a/src/test/regress/sql/create_am.sql+++ b/src/test/regress/sql/create_am.sql@@ -124,8 +124,11 @@ 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 .. PARTITION BY supports USING.
CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a) USING heap2;
+SELECT a.amname FROM pg_class c, pg_am a+ WHERE c.relname = 'tableam_parted_heap2' AND a.oid = c.relam;+DROP TABLE tableam_parted_heap2;
CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a);
-- new partitions will inherit from the current default, rather the partition root
@@ -213,11 +216,91 @@ ALTER TABLE heaptable SET ACCESS METHOD DEFAULT, SET ACCESS METHOD heap2;
ALTER MATERIALIZED VIEW heapmv SET ACCESS METHOD heap, SET ACCESS METHOD heap2;
DROP MATERIALIZED VIEW heapmv;
DROP TABLE heaptable;
--- No support for partitioned tables.-CREATE TABLE am_partitioned(x INT, y INT)- PARTITION BY hash (x);++-- Partition hierarchies with access methods+BEGIN;+SET LOCAL default_table_access_method = 'heap';+CREATE TABLE am_partitioned(x INT, y INT) PARTITION BY hash (x);+-- pg_class.relam is 0, no dependency recorded between the AM and the+-- partitioned table.+SELECT relam FROM pg_class WHERE relname = 'am_partitioned';+SELECT pg_describe_object(classid, objid, objsubid) AS obj,+ pg_describe_object(refclassid, refobjid, refobjsubid) as refobj+ FROM pg_depend, pg_am+ WHERE pg_depend.refclassid = 'pg_am'::regclass+ AND pg_am.oid = pg_depend.refobjid+ AND pg_depend.objid = 'am_partitioned'::regclass;+-- New default is set, with dependency added.+ALTER TABLE am_partitioned SET ACCESS METHOD heap2;+SELECT a.amname FROM pg_class c, pg_am a+ WHERE c.relname = 'am_partitioned' AND a.oid = c.relam;+SELECT pg_describe_object(classid, objid, objsubid) AS obj,+ pg_describe_object(refclassid, refobjid, refobjsubid) as refobj+ FROM pg_depend, pg_am+ WHERE pg_depend.refclassid = 'pg_am'::regclass+ AND pg_am.oid = pg_depend.refobjid+ AND pg_depend.objid = 'am_partitioned'::regclass;+-- Default is set, with dependency updated.+SET LOCAL default_table_access_method = 'heap2';+ALTER TABLE am_partitioned SET ACCESS METHOD heap;+SELECT a.amname FROM pg_class c, pg_am a+ WHERE c.relname = 'am_partitioned' AND a.oid = c.relam;+-- Dependency pinned, hence removed.+SELECT pg_describe_object(classid, objid, objsubid) AS obj,+ pg_describe_object(refclassid, refobjid, refobjsubid) as refobj+ FROM pg_depend, pg_am+ WHERE pg_depend.refclassid = 'pg_am'::regclass+ AND pg_am.oid = pg_depend.refobjid+ AND pg_depend.objid = 'am_partitioned'::regclass;+-- Default and AM set in in clause are the same, relam should be set.+SET LOCAL default_table_access_method = 'heap2';+ALTER TABLE am_partitioned SET ACCESS METHOD heap2;+SELECT a.amname FROM pg_class c, pg_am a+ WHERE c.relname = 'am_partitioned' AND a.oid = c.relam;+-- Reset to default+ALTER TABLE am_partitioned SET ACCESS METHOD DEFAULT;+SELECT relam FROM pg_class WHERE relname = 'am_partitioned';+-- Upon ALTER TABLE SET ACCESS METHOD on a partitioned table, new partitions+-- will inherit the AM set. Existing partitioned are unchanged.+SELECT relam FROM pg_class WHERE relname = 'am_partitioned';+SET LOCAL default_table_access_method = 'heap';+CREATE TABLE am_partitioned_0 PARTITION OF am_partitioned+ FOR VALUES WITH (MODULUS 10, REMAINDER 0);+SET LOCAL default_table_access_method = 'heap2';+CREATE TABLE am_partitioned_1 PARTITION OF am_partitioned+ FOR VALUES WITH (MODULUS 10, REMAINDER 1);+SET LOCAL default_table_access_method = 'heap';+ALTER TABLE am_partitioned SET ACCESS METHOD heap2;+CREATE TABLE am_partitioned_2 PARTITION OF am_partitioned+ FOR VALUES WITH (MODULUS 10, REMAINDER 2);+ALTER TABLE am_partitioned SET ACCESS METHOD DEFAULT;+SELECT relam FROM pg_class WHERE relname = 'am_partitioned';+CREATE TABLE am_partitioned_3 PARTITION OF am_partitioned+ FOR VALUES WITH (MODULUS 10, REMAINDER 3);+-- Partitioned table with relam at 0+ALTER TABLE am_partitioned SET ACCESS METHOD DEFAULT;+CREATE TABLE am_partitioned_5p PARTITION OF am_partitioned+ FOR VALUES WITH (MODULUS 10, REMAINDER 5) PARTITION BY hash(y);+-- Partitions of this partitioned table inherit default AM at creation+-- time.+CREATE TABLE am_partitioned_5p1 PARTITION OF am_partitioned_5p+ FOR VALUES WITH (MODULUS 10, REMAINDER 1);+-- Partitioned table with relam set.
ALTER TABLE am_partitioned SET ACCESS METHOD heap2;
+CREATE TABLE am_partitioned_6p PARTITION OF am_partitioned+ FOR VALUES WITH (MODULUS 10, REMAINDER 6) PARTITION BY hash(y);+-- Partitions of this partitioned table inherit its AM.+CREATE TABLE am_partitioned_6p1 PARTITION OF am_partitioned_6p+ FOR VALUES WITH (MODULUS 10, REMAINDER 1);+SELECT c.relname, a.amname FROM pg_class c, pg_am a+ WHERE c.relam = a.oid AND+ c.relname LIKE 'am_partitioned%'+UNION ALL+SELECT c.relname, 'default' FROM pg_class c+ WHERE c.relam = 0+ AND c.relname LIKE 'am_partitioned%' ORDER BY 1;
DROP TABLE am_partitioned;
+COMMIT;
-- Second, create objects in the new AM by changing the default AM
BEGIN;
--
2.42.0
0002-f.patch (text/x-diff, 6.7K) download
From a69348b59307ab85a051ea17aabca3150a391eae Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 2 Mar 2024 07:23:00 -0600
Subject: [PATCH 2/2] f
---
src/backend/commands/tablecmds.c | 74 +++++++++++---------------------
1 file changed, 25 insertions(+), 49 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex cf595329b6c..3f26379b1bd 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -183,8 +183,8 @@ 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 */- bool defaultAccessMethod; /* true if SET ACCESS METHOD DEFAULT */+ bool chgAccessMethod; /* T if SET ACCESS METHOD is used */+ Oid newAccessMethod; /* new access method; 0 means no change - if above is true */
Oid newTableSpace; /* new tablespace; 0 means no change */
bool chgPersistence; /* T if SET LOGGED/UNLOGGED is used */
char newrelpersistence; /* if above is true */
@@ -590,8 +590,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,- bool defaultAccessMethod);+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);
@@ -5053,7 +5052,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_MATVIEW);
/* check if another access method change was already requested */
- if (OidIsValid(tab->newAccessMethod))+ if (tab->chgAccessMethod)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot have multiple SET ACCESS METHOD subcommands")));
@@ -5413,9 +5412,8 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
* Only do this for partitioned tables, for which this is just a
* catalog change. Tables with storage are handled by Phase 3.
*/
- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)- ATExecSetAccessMethodNoStorage(rel, tab->newAccessMethod,- tab->defaultAccessMethod);+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE && tab->chgAccessMethod)+ ATExecSetAccessMethodNoStorage(rel, tab->newAccessMethod);
break;
case AT_SetTableSpace: /* SET TABLESPACE */
@@ -5821,7 +5819,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
* Select destination access method (same as original unless user
* requested a change)
*/
- if (OidIsValid(tab->newAccessMethod))+ if (tab->chgAccessMethod)
NewAccessMethod = tab->newAccessMethod;
else
NewAccessMethod = OldHeap->rd_rel->relam;
@@ -6408,7 +6406,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newAccessMethod = InvalidOid;
- tab->defaultAccessMethod = false;+ tab->chgAccessMethod = false;
tab->newTableSpace = InvalidOid;
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
tab->chgPersistence = false;
@@ -15225,39 +15223,25 @@ ATPrepSetAccessMethod(AlteredTableInfo *tab, Relation rel, const char *amname)
{
Oid amoid;
- /* Check that the table access method exists */- amoid = get_table_am_oid(amname ? amname : default_table_access_method,- false);+ /*+ * Check that the table access method exists.+ * Use the access method, if specified, otherwise (when not specified) 0+ * for partitioned tables or the configured default AM.+ */+ if (amname != NULL)+ amoid = get_table_am_oid(amname, false);+ else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)+ amoid = 0;+ else+ amoid = get_table_am_oid(default_table_access_method, false);- if (RELKIND_HAS_STORAGE(rel->rd_rel->relkind) &&- rel->rd_rel->relam == amoid)+ if (rel->rd_rel->relam == amoid)
return;
- /* Partitioned tables are handled here */- if (!RELKIND_HAS_STORAGE(rel->rd_rel->relkind))- {- if (amname == NULL)- {- /* DEFAULT implied, leave if relam is already InvalidOid */- if (!OidIsValid(rel->rd_rel->relam))- return;- }- else- {- /*- * Access method set by query, leave if only this matches with its- * relam.- */- if (OidIsValid(rel->rd_rel->relam) &&- amoid == rel->rd_rel->relam)- return;- }- }-
/* Save info for Phase 3 to do the real work */
tab->rewrite |= AT_REWRITE_ACCESS_METHOD;
tab->newAccessMethod = amoid;
- tab->defaultAccessMethod = (amname == NULL);+ tab->chgAccessMethod = true;
}
/*
@@ -15587,12 +15571,10 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
* storage that have an interest in preserving AM.
*
* Since these have no storage, setting the access method is a catalog only
- * operation. defaultAccessMethod tracks if a default value is wanted for- * the access method.+ * operation.
*/
static void
-ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethodId,- bool defaultAccessMethod)+ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethodId)
{
Relation pg_class;
Oid oldAccessMethodId;
@@ -15600,12 +15582,6 @@ ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethodId,
Form_pg_class rd_rel;
Oid reloid = RelationGetRelid(rel);
- if (!OidIsValid(newAccessMethodId))- {- /* if the access method is unchanged, leave */- return;- }-
/*
* Shouldn't be called on relations having storage; these are processed in
* phase 3.
@@ -15622,7 +15598,7 @@ ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethodId,
/* Update the pg_class row. */
oldAccessMethodId = rd_rel->relam;
- rd_rel->relam = defaultAccessMethod ? InvalidOid : newAccessMethodId;+ rd_rel->relam = newAccessMethodId;
/* Leave if no update required */
if (rd_rel->relam == oldAccessMethodId)
@@ -15640,7 +15616,7 @@ ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethodId,
* that this has to compare the previous value stored in pg_class with the
* new one.
*/
- if (!OidIsValid(oldAccessMethodId) && OidIsValid(rd_rel->relam))+ if (!OidIsValid(oldAccessMethodId) && OidIsValid(newAccessMethodId))
{
ObjectAddress relobj,
referenced;
--
2.42.0
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, michael@paquier.xyz, peter@eisentraut.org, soumyadeep2007@gmail.com, zyu@yugabyte.com, ashwinstar@gmail.com, vanjared@vmware.com
Subject: Re: ALTER TABLE SET ACCESS METHOD on partitioned tables
In-Reply-To: <ZepxmAvTNYfptrQi@pryzbyj2023>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox