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()
