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
>From 8aca5e443ebb41b7de1ba18b519a33a97a41a897 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@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.sgml
index 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.sgml
index 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.sgml
index 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.c
index 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.c
index 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.c
index 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.c
index 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.pl
index 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.h
index 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.h
index 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.out
index 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.sql
index 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
>From a69348b59307ab85a051ea17aabca3150a391eae Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@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.c
index 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
