Re: ALTER TABLE SET ACCESS METHOD on partitioned tables

[Justin Pryzby]

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