Re: alter table set TABLE ACCESS METHOD

On Tue, Jul 27, 2021 at 08:40:59PM -0700, Jeff Davis wrote:
> I just returned from vacation and I was about ready to commit this
> myself, but I noticed that it doesn't seem to be calling
> InvokeObjectPostAlterHook().

Arg, sorry about that!  I was unclear what the situation of the patch
was.

> I was in the process of trying to be sure
> of where to call it. It looks like it should be called after catalog
> changes but before CommandCounterIncrement(), and it also looks like it
> should be called even for no-op commands.

Right.  Isn't that an older issue though?  A rewrite involved after a
change of relpersistence does not call the hook either.  It looks to
me that this should be after finish_heap_swap() to match with
ATExecSetTableSpace() in ATRewriteTables().  The only known user of
object_access_hook in the core code is sepgsql, so this would
involve a change of behavior.  And I don't recall any backpatching
that added a post-alter hook.

> Also, I agree with Justin that it should fail when there are multiple
> SET ACCESS METHOD subcommands consistently, regardless of whether one
> is a no-op, and it should probably throw a syntax error to match SET
> TABLESPACE.

Hmm.  Okay.

> Minor nit: in tab-complete.c, why does it say "<smt>"? Is that just a
> typo or is there a reason it's different from everything else, which
> uses "<sth>"? And what does "sth" mean anyway?

"Something".  That should be "<sth>" to be consistent with the area.
--
Michael