Re: alter table set TABLE ACCESS METHOD

On Wed, Jun 09, 2021 at 01:45:52PM -0700, Zhihong Yu wrote:
> +           /* check if another access method change was already requested
> */
> +           if (tab->newAccessMethod)
> +               ereport(ERROR,
> +                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> +                        errmsg("cannot change access method setting twice")));
> 
> I think the error message can be refined - changing  access method twice is
> supported, as long as the two changes don't overlap.

That language is consistent wtih existing errors.

src/backend/commands/tablecmds.c:                                                errmsg("cannot change persistence setting twice")));
src/backend/commands/tablecmds.c:                                                errmsg("cannot change persistence setting twice")));
                                 errmsg("cannot alter type of column \"%s\" twice",

However, I think that SET TABLESPACE is a better template to follow:
                                 errmsg("cannot have multiple SET TABLESPACE subcommands")));

Michael pointed out that there's two, redundant checks:

+       if (rel->rd_rel->relam == amoid)
+               return;
+  
+       /* Save info for Phase 3 to do the real work */
+       if (OidIsValid(tab->newAccessMethod))
+               ereport(ERROR,
+                               (errcode(ERRCODE_SYNTAX_ERROR),
+                                errmsg("cannot have multiple SET ACCESS METHOD subcommands")));

I think that the "multiple subcommands" test should be before the "no-op" test.

@Jeff: In my original patch, I also proposed patches 2,3:

Subject: [PATCH v2 2/3] Allow specifying acccess method of partitioned tables..
Subject: [PATCH v2 3/3] Implement lsyscache get_rel_relam()