On Wed, Jul 28, 2021 at 01:05:10PM -0700, Jeff Davis wrote: > On Wed, 2021-07-28 at 14:02 +0900, Michael Paquier wrote: >> 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. > > Sounds like it should be a different patch. Thank you.
Doing any checks around the hooks of objectaccess.h is very annoying, because we have no modules to check after them easily except sepgsql. Anyway, I have been checking that, with the hack-ish module attached and tracked down that swap_relation_files() calls InvokeObjectPostAlterHookArg() already (as you already spotted?), but that's an internal change when it comes to SET LOGGED/UNLOGGED/ACCESS METHOD :( Attached is a small module I have used for those tests, for reference. It passes on HEAD, and with the patch attached you can see the extra entries. >>> 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. I'd still disagree with that. One example is SET LOGGED that would not fail when repeated multiple times. Also, tracking down if a SET ACCESS METHOD subcommand has been passed down requires an extra field in each tab entry so I am not sure that this is worth the extra complication. I can see benefits and advantages one way or the other, and I would tend to keep the code a maximum simple as we never store InvalidOid for a table AM. Anyway, I won't fight the majority either. >>> 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. > > These two issues are pretty minor. Fixed this one, while not forgetting about it. Thanks. -- Michael
object_hooks.tar.gz
Description: application/gzip
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index fcd778c62a..b18de38e73 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5475,6 +5475,8 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
RecentXmin,
ReadNextMultiXactId(),
persistence);
+
+ InvokeObjectPostAlterHook(RelationRelationId, tab->relid, 0);
}
else
{
signature.asc
Description: PGP signature
