Re: ALTER TABLE SET ACCESS METHOD on partitioned tables - Mailing list pgsql-hackers

On Thu, Mar 21, 2024 at 01:07:01PM +0100, Alvaro Herrera wrote:
> Given that Michaël is temporarily gone, I propose to push the attached
> tomorrow.

Thanks.

On Tue, Mar 26, 2024 at 12:05:47PM +0100, Alvaro Herrera wrote:
> On 2024-Mar-26, Alexander Lakhin wrote:
> 
> > Hello Alvaro,
> > 
> > 21.03.2024 15:07, Alvaro Herrera wrote:
> > > Given that Michaël is temporarily gone, I propose to push the attached
> > > tomorrow.
> > 
> > Please look at a new anomaly introduced with 374c7a229.
> > Starting from that commit, the following erroneous query:
> > CREATE FOREIGN TABLE fp PARTITION OF pg_am DEFAULT SERVER x;
> > 
> > triggers an assertion failure:
> > TRAP: failed Assert("relation->rd_rel->relam == InvalidOid"), File: "relcache.c", Line: 1219, PID: 3706301
> 
> Hmm, yeah, we're setting relam for relations that shouldn't have it.
> I propose the attached.

Looks right.  That's how I originally wrote it, except for the
"stmt->accessMethod != NULL" case.

I prefered my way - the grammar should refuse to set stmt->accessMethod
for inappropriate relkinds.  And you could assert that.

I also prefered to set "accessMethodId = InvalidOid" once, rather than
twice.

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8a02c5b05b6..050be89728f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -962,18 +962,21 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
      * case of a partitioned table) the parent's, if it has one.
      */
     if (stmt->accessMethod != NULL)
-        accessMethodId = get_table_am_oid(stmt->accessMethod, false);
-    else if (stmt->partbound)
     {
-        Assert(list_length(inheritOids) == 1);
-        accessMethodId = get_rel_relam(linitial_oid(inheritOids));
+        Assert(RELKIND_HAS_TABLE_AM(relkind) || relkind == RELKIND_PARTITIONED_TABLE);
+        accessMethodId = get_table_am_oid(stmt->accessMethod, false);
     }
-    else
-        accessMethodId = InvalidOid;
+    else if (RELKIND_HAS_TABLE_AM(relkind) || relkind == RELKIND_PARTITIONED_TABLE)
+    {
+        if (stmt->partbound)
+        {
+            Assert(list_length(inheritOids) == 1);
+            accessMethodId = get_rel_relam(linitial_oid(inheritOids));
+        }
 
-    /* still nothing? use the default */
-    if (RELKIND_HAS_TABLE_AM(relkind) && !OidIsValid(accessMethodId))
-        accessMethodId = get_table_am_oid(default_table_access_method, false);
+        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