Re: alter table set TABLE ACCESS METHOD

[Michael Paquier]

On Fri, Jul 30, 2021 at 02:18:02PM -0700, Jeff Davis wrote:
> It sounds like anything we do here should be part of a larger change to
> make it consistent. So I'm fine with the patch you posted.

As a matter of curiosity, I have checked how it would look to handle
the no-op case for the sub-commands other than SET TABLESPACE, and one
would need something like the attached, with a new flag for
AlteredTableInfo.  That does not really look good, but it triggers
properly the object access hook when SET LOGGED/UNLOGGED/ACCESS METHOD
are no-ops, so let's just handle the case using the version from
upthread.  I'll do that at the beginning of next week.
--
Michael

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index fcd778c62a..eecbde8479 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -179,6 +179,8 @@ typedef struct AlteredTableInfo
 	Oid			newAccessMethod;	/* new access method; 0 means no change */
 	Oid			newTableSpace;	/* new tablespace; 0 means no change */
 	bool		chgPersistence; /* T if SET LOGGED/UNLOGGED is used */
+	bool		rewriteAttempted;	/* T if a rewrite operation was attempted,
+									 * may be a no-op */
 	char		newrelpersistence;	/* if above is true */
 	Expr	   *partition_constraint;	/* for attach partition validation */
 	/* true, if validating default due to some other attach/detach */
@@ -4593,6 +4595,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
 			break;
 		case AT_SetLogged:		/* SET LOGGED */
 			ATSimplePermissions(cmd->subtype, rel, ATT_TABLE);
+			tab->rewriteAttempted = true;
 			if (tab->chgPersistence)
 				ereport(ERROR,
 						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -4608,6 +4611,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
 			break;
 		case AT_SetUnLogged:	/* SET UNLOGGED */
 			ATSimplePermissions(cmd->subtype, rel, ATT_TABLE);
+			tab->rewriteAttempted = true;
 			if (tab->chgPersistence)
 				ereport(ERROR,
 						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -5475,6 +5479,8 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
 							 RecentXmin,
 							 ReadNextMultiXactId(),
 							 persistence);
+
+			InvokeObjectPostAlterHook(RelationRelationId, tab->relid, 0);
 		}
 		else
 		{
@@ -5493,6 +5499,16 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
 			 */
 			if (tab->newTableSpace)
 				ATExecSetTableSpace(tab->relid, tab->newTableSpace, lockmode);
+			else if (tab->rewriteAttempted)
+			{
+				/*
+				 * Even if no rewrite is going to happen, it may be possible
+				 * that one has gone through SET LOGGED/UNLOGGED or ACCESS
+				 * METHOD while being a no-op.  If that's the case, invoke the
+				 * access hook.
+				 */
+				InvokeObjectPostAlterHook(RelationRelationId, tab->relid, 0);
+			}
 		}
 	}
 
@@ -5971,6 +5987,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
 	tab->newTableSpace = InvalidOid;
 	tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
 	tab->chgPersistence = false;
+	tab->rewriteAttempted = false;
 
 	*wqueue = lappend(*wqueue, tab);
 
@@ -13660,6 +13677,7 @@ ATPrepSetAccessMethod(AlteredTableInfo *tab, Relation rel, const char *amname)
 
 	/* Check that the table access method exists */
 	amoid = get_table_am_oid(amname, false);
+	tab->rewriteAttempted = true;
 
 	if (rel->rd_rel->relam == amoid)
 		return;

signature.asc
Description: PGP signature