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
signature.asc
Description: PGP signature
