Tom Lane wrote:
> Alvaro Herrera <alvhe...@2ndquadrant.com> writes:
> > Agreed.  I am going over this patch, and the last bit I need to sort out
> > is the wording of these messages.  I have temporarily settled on this:
> 
> >     ereport(ERROR,
> >                     (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
> >                      errmsg("cannot change logged status of table %s to 
> > logged",
> >                                     RelationGetRelationName(rel)),
> >                      errdetail("Table %s references unlogged table %s.",
> >                                        RelationGetRelationName(rel),
> >                                        RelationGetRelationName(relfk))));
> 
> > Note the term "logged status" to talk about whether a table is logged or
> > not.  I thought about "loggedness" but I'm not sure english speakers are
> > going to love me for that.  Any other ideas there?
> 
> Just say "cannot change status of table %s to logged".

I like this one, thanks.

> > Yeah, there is precedent for silently doing nothing.  We previously
> > threw warnings or notices, but nowadays even that is gone.  Throwing an
> > error definitely seems the wrong thing.
> 
> Agreed, just do nothing if it's already the right setting.

Done that way.

Andres Freund wrote:

> Have you looked at the correctness of the patch itself? Last time I'd
> looked it has fundamental correctness issues. I'd outlined a possible
> solution, but I haven't looked since.

Yeah, Fabrizio had it passing the relpersistence down to make_new_heap,
so the transient table is created with the right setting.  AFAICS it's
good now.  I'm a bit uneasy about the way it changes indexes: it just
updates pg_class for them just before invoking the reindex in
finish_heap_swap.  I think it's correct as it stands though; at least,
the rewrite phase happens with the right setting, so that if there are
constraints being checked and these invoke index scans, such accesses
would not leave buffers with the wrong setting in shared_buffers.

Another option would be to pass the new relpersistence down to
finish_heap_swap, but that would be hugely complicated AFAICS.

Here's the updated patch.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
*** a/doc/src/sgml/ref/alter_table.sgml
--- b/doc/src/sgml/ref/alter_table.sgml
***************
*** 61,66 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
--- 61,68 ----
      SET WITHOUT CLUSTER
      SET WITH OIDS
      SET WITHOUT OIDS
+     SET TABLESPACE <replaceable class="PARAMETER">new_tablespace</replaceable>
+     SET {LOGGED | UNLOGGED}
      SET ( <replaceable class="PARAMETER">storage_parameter</replaceable> = <replaceable class="PARAMETER">value</replaceable> [, ... ] )
      RESET ( <replaceable class="PARAMETER">storage_parameter</replaceable> [, ... ] )
      INHERIT <replaceable class="PARAMETER">parent_table</replaceable>
***************
*** 68,74 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
      OF <replaceable class="PARAMETER">type_name</replaceable>
      NOT OF
      OWNER TO <replaceable class="PARAMETER">new_owner</replaceable>
-     SET TABLESPACE <replaceable class="PARAMETER">new_tablespace</replaceable>
      REPLICA IDENTITY {DEFAULT | USING INDEX <replaceable class="PARAMETER">index_name</replaceable> | FULL | NOTHING}
  
  <phrase>and <replaceable class="PARAMETER">table_constraint_using_index</replaceable> is:</phrase>
--- 70,75 ----
***************
*** 477,482 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
--- 478,508 ----
     </varlistentry>
  
     <varlistentry>
+     <term><literal>SET TABLESPACE</literal></term>
+     <listitem>
+      <para>
+       This form changes the table's tablespace to the specified tablespace and
+       moves the data file(s) associated with the table to the new tablespace.
+       Indexes on the table, if any, are not moved; but they can be moved
+       separately with additional <literal>SET TABLESPACE</literal> commands.
+       See also
+       <xref linkend="SQL-CREATETABLESPACE">.
+      </para>
+     </listitem>
+    </varlistentry>
+ 
+    <varlistentry>
+     <term><literal>SET {LOGGED | UNLOGGED}</literal></term>
+     <listitem>
+      <para>
+       This form changes the table from unlogged to logged or vice-versa
+       (see <xref linkend="SQL-CREATETABLE-UNLOGGED">).  It cannot be applied
+       to a temporary table.
+      </para>
+     </listitem>
+    </varlistentry>
+ 
+    <varlistentry>
      <term><literal>SET ( <replaceable class="PARAMETER">storage_parameter</replaceable> = <replaceable class="PARAMETER">value</replaceable> [, ... ] )</literal></term>
      <listitem>
       <para>
***************
*** 589,608 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
      </listitem>
     </varlistentry>
  
-    <varlistentry>
-     <term><literal>SET TABLESPACE</literal></term>
-     <listitem>
-      <para>
-       This form changes the table's tablespace to the specified tablespace and
-       moves the data file(s) associated with the table to the new tablespace.
-       Indexes on the table, if any, are not moved; but they can be moved
-       separately with additional <literal>SET TABLESPACE</literal> commands.
-       See also
-       <xref linkend="SQL-CREATETABLESPACE">.
-      </para>
-     </listitem>
-    </varlistentry>
- 
     <varlistentry id="SQL-CREATETABLE-REPLICA-IDENTITY">
      <term><literal>REPLICA IDENTITY</literal></term>
      <listitem>
--- 615,620 ----
*** a/src/backend/commands/cluster.c
--- b/src/backend/commands/cluster.c
***************
*** 574,580 **** rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
  	heap_close(OldHeap, NoLock);
  
  	/* Create the transient table that will receive the re-ordered data */
! 	OIDNewHeap = make_new_heap(tableOid, tableSpace, false,
  							   AccessExclusiveLock);
  
  	/* Copy the heap data into the new table in the desired order */
--- 574,581 ----
  	heap_close(OldHeap, NoLock);
  
  	/* Create the transient table that will receive the re-ordered data */
! 	OIDNewHeap = make_new_heap(tableOid, tableSpace,
! 							   OldHeap->rd_rel->relpersistence,
  							   AccessExclusiveLock);
  
  	/* Copy the heap data into the new table in the desired order */
***************
*** 595,607 **** rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
   * Create the transient table that will be filled with new data during
   * CLUSTER, ALTER TABLE, and similar operations.  The transient table
   * duplicates the logical structure of the OldHeap, but is placed in
!  * NewTableSpace which might be different from OldHeap's.
   *
   * After this, the caller should load the new heap with transferred/modified
   * data, then call finish_heap_swap to complete the operation.
   */
  Oid
! make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, bool forcetemp,
  			  LOCKMODE lockmode)
  {
  	TupleDesc	OldHeapDesc;
--- 596,609 ----
   * Create the transient table that will be filled with new data during
   * CLUSTER, ALTER TABLE, and similar operations.  The transient table
   * duplicates the logical structure of the OldHeap, but is placed in
!  * NewTableSpace which might be different from OldHeap's.  Also, it's built
!  * with the specified persistence, which might differ from the original's.
   *
   * After this, the caller should load the new heap with transferred/modified
   * data, then call finish_heap_swap to complete the operation.
   */
  Oid
! make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, char relpersistence,
  			  LOCKMODE lockmode)
  {
  	TupleDesc	OldHeapDesc;
***************
*** 613,619 **** make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, bool forcetemp,
  	Datum		reloptions;
  	bool		isNull;
  	Oid			namespaceid;
- 	char		relpersistence;
  
  	OldHeap = heap_open(OIDOldHeap, lockmode);
  	OldHeapDesc = RelationGetDescr(OldHeap);
--- 615,620 ----
***************
*** 636,651 **** make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, bool forcetemp,
  	if (isNull)
  		reloptions = (Datum) 0;
  
! 	if (forcetemp)
! 	{
  		namespaceid = LookupCreationNamespace("pg_temp");
- 		relpersistence = RELPERSISTENCE_TEMP;
- 	}
  	else
- 	{
  		namespaceid = RelationGetNamespace(OldHeap);
- 		relpersistence = OldHeap->rd_rel->relpersistence;
- 	}
  
  	/*
  	 * Create the new heap, using a temporary name in the same namespace as
--- 637,646 ----
  	if (isNull)
  		reloptions = (Datum) 0;
  
! 	if (relpersistence == RELPERSISTENCE_TEMP)
  		namespaceid = LookupCreationNamespace("pg_temp");
  	else
  		namespaceid = RelationGetNamespace(OldHeap);
  
  	/*
  	 * Create the new heap, using a temporary name in the same namespace as
***************
*** 1109,1116 **** copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
  /*
   * Swap the physical files of two given relations.
   *
!  * We swap the physical identity (reltablespace and relfilenode) while
!  * keeping the same logical identities of the two relations.
   *
   * We can swap associated TOAST data in either of two ways: recursively swap
   * the physical content of the toast tables (and their indexes), or swap the
--- 1104,1113 ----
  /*
   * Swap the physical files of two given relations.
   *
!  * We swap the physical identity (reltablespace, relfilenode) while keeping the
!  * same logical identities of the two relations.  relpersistence is also
!  * swapped, which is critical since it determines where buffers live for each
!  * relation.
   *
   * We can swap associated TOAST data in either of two ways: recursively swap
   * the physical content of the toast tables (and their indexes), or swap the
***************
*** 1146,1151 **** swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
--- 1143,1149 ----
  	Oid			relfilenode1,
  				relfilenode2;
  	Oid			swaptemp;
+ 	char		swptmpchr;
  	CatalogIndexState indstate;
  
  	/* We need writable copies of both pg_class tuples. */
***************
*** 1166,1172 **** swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
  
  	if (OidIsValid(relfilenode1) && OidIsValid(relfilenode2))
  	{
! 		/* Normal non-mapped relations: swap relfilenodes and reltablespaces */
  		Assert(!target_is_pg_class);
  
  		swaptemp = relform1->relfilenode;
--- 1164,1173 ----
  
  	if (OidIsValid(relfilenode1) && OidIsValid(relfilenode2))
  	{
! 		/*
! 		 * Normal non-mapped relations: swap relfilenodes, reltablespaces,
! 		 * relpersistence
! 		 */
  		Assert(!target_is_pg_class);
  
  		swaptemp = relform1->relfilenode;
***************
*** 1177,1182 **** swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
--- 1178,1187 ----
  		relform1->reltablespace = relform2->reltablespace;
  		relform2->reltablespace = swaptemp;
  
+ 		swptmpchr = relform1->relpersistence;
+ 		relform1->relpersistence = relform2->relpersistence;
+ 		relform2->relpersistence = swptmpchr;
+ 
  		/* Also swap toast links, if we're swapping by links */
  		if (!swap_toast_by_content)
  		{
***************
*** 1196,1210 **** swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
  				 NameStr(relform1->relname));
  
  		/*
! 		 * We can't change the tablespace of a mapped rel, and we can't handle
! 		 * toast link swapping for one either, because we must not apply any
! 		 * critical changes to its pg_class row.  These cases should be
! 		 * prevented by upstream permissions tests, so this check is a
! 		 * non-user-facing emergency backstop.
  		 */
  		if (relform1->reltablespace != relform2->reltablespace)
  			elog(ERROR, "cannot change tablespace of mapped relation \"%s\"",
  				 NameStr(relform1->relname));
  		if (!swap_toast_by_content &&
  			(relform1->reltoastrelid || relform2->reltoastrelid))
  			elog(ERROR, "cannot swap toast by links for mapped relation \"%s\"",
--- 1201,1218 ----
  				 NameStr(relform1->relname));
  
  		/*
! 		 * We can't change the tablespace nor persistence of a mapped rel, and
! 		 * we can't handle toast link swapping for one either, because we must
! 		 * not apply any critical changes to its pg_class row.  These cases
! 		 * should be prevented by upstream permissions tests, so these checks
! 		 * are non-user-facing emergency backstop.
  		 */
  		if (relform1->reltablespace != relform2->reltablespace)
  			elog(ERROR, "cannot change tablespace of mapped relation \"%s\"",
  				 NameStr(relform1->relname));
+ 		if (relform1->relpersistence != relform2->relpersistence)
+ 			elog(ERROR, "cannot change persistence of mapped relation \"%s\"",
+ 				 NameStr(relform1->relname));
  		if (!swap_toast_by_content &&
  			(relform1->reltoastrelid || relform2->reltoastrelid))
  			elog(ERROR, "cannot swap toast by links for mapped relation \"%s\"",
*** a/src/backend/commands/matview.c
--- b/src/backend/commands/matview.c
***************
*** 147,152 **** ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
--- 147,153 ----
  	DestReceiver *dest;
  	bool		concurrent;
  	LOCKMODE	lockmode;
+ 	char		relpersistence;
  
  	/* Determine strength of lock needed. */
  	concurrent = stmt->concurrent;
***************
*** 233,241 **** ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
--- 234,248 ----
  
  	/* Concurrent refresh builds new data in temp tablespace, and does diff. */
  	if (concurrent)
+ 	{
  		tableSpace = GetDefaultTablespace(RELPERSISTENCE_TEMP);
+ 		relpersistence = RELPERSISTENCE_TEMP;
+ 	}
  	else
+ 	{
  		tableSpace = matviewRel->rd_rel->reltablespace;
+ 		relpersistence = matviewRel->rd_rel->relpersistence;
+ 	}
  
  	owner = matviewRel->rd_rel->relowner;
  
***************
*** 244,250 **** ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
  	 * it against access by any other process until commit (by which time it
  	 * will be gone).
  	 */
! 	OIDNewHeap = make_new_heap(matviewOid, tableSpace, concurrent,
  							   ExclusiveLock);
  	LockRelationOid(OIDNewHeap, AccessExclusiveLock);
  	dest = CreateTransientRelDestReceiver(OIDNewHeap);
--- 251,257 ----
  	 * it against access by any other process until commit (by which time it
  	 * will be gone).
  	 */
! 	OIDNewHeap = make_new_heap(matviewOid, tableSpace, relpersistence,
  							   ExclusiveLock);
  	LockRelationOid(OIDNewHeap, AccessExclusiveLock);
  	dest = CreateTransientRelDestReceiver(OIDNewHeap);
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 151,156 **** typedef struct AlteredTableInfo
--- 151,158 ----
  	bool		new_notnull;	/* T if we added new NOT NULL constraints */
  	bool		rewrite;		/* T if a rewrite is forced */
  	Oid			newTableSpace;	/* new tablespace; 0 means no change */
+ 	bool		chgLoggedness;	/* T if SET LOGGED/UNLOGGED is used */
+ 	char		newrelpersistence;		/* if above is true */
  	/* Objects to rebuild after completing ALTER TYPE operations */
  	List	   *changedConstraintOids;	/* OIDs of constraints to rebuild */
  	List	   *changedConstraintDefs;	/* string definitions of same */
***************
*** 371,377 **** static void ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
  					  AlterTableCmd *cmd, LOCKMODE lockmode);
  static void ATExecAlterColumnGenericOptions(Relation rel, const char *colName,
  								List *options, LOCKMODE lockmode);
! static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode);
  static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId,
  					 char *cmd, List **wqueue, LOCKMODE lockmode,
  					 bool rewrite);
--- 373,380 ----
  					  AlterTableCmd *cmd, LOCKMODE lockmode);
  static void ATExecAlterColumnGenericOptions(Relation rel, const char *colName,
  								List *options, LOCKMODE lockmode);
! static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab,
! 					   LOCKMODE lockmode);
  static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId,
  					 char *cmd, List **wqueue, LOCKMODE lockmode,
  					 bool rewrite);
***************
*** 381,388 **** static void change_owner_fix_column_acls(Oid relationOid,
  							 Oid oldOwnerId, Oid newOwnerId);
  static void change_owner_recurse_to_sequences(Oid relationOid,
  								  Oid newOwnerId, LOCKMODE lockmode);
! static void ATExecClusterOn(Relation rel, const char *indexName, LOCKMODE lockmode);
  static void ATExecDropCluster(Relation rel, LOCKMODE lockmode);
  static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
  					char *tablespacename, LOCKMODE lockmode);
  static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode);
--- 384,394 ----
  							 Oid oldOwnerId, Oid newOwnerId);
  static void change_owner_recurse_to_sequences(Oid relationOid,
  								  Oid newOwnerId, LOCKMODE lockmode);
! static void ATExecClusterOn(Relation rel, const char *indexName,
! 				LOCKMODE lockmode);
  static void ATExecDropCluster(Relation rel, LOCKMODE lockmode);
+ static bool ATPrepChangeLoggedness(Relation rel, bool toLogged);
+ static void ATChangeIndexesLoggedness(Oid relid, char relpersistence);
  static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
  					char *tablespacename, LOCKMODE lockmode);
  static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode);
***************
*** 2948,2953 **** AlterTableGetLockLevel(List *cmds)
--- 2954,2964 ----
  				cmd_lockmode = ShareUpdateExclusiveLock;
  				break;
  
+ 			case AT_SetLogged:
+ 			case AT_SetUnLogged:
+ 				cmd_lockmode = AccessExclusiveLock;
+ 				break;
+ 
  			case AT_ValidateConstraint: /* Uses MVCC in
  												 * getConstraints() */
  				cmd_lockmode = ShareUpdateExclusiveLock;
***************
*** 3160,3165 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
--- 3171,3194 ----
  			/* No command-specific prep needed */
  			pass = AT_PASS_MISC;
  			break;
+ 		case AT_SetLogged:		/* SET LOGGED */
+ 			ATSimplePermissions(rel, ATT_TABLE);
+ 			tab->chgLoggedness = ATPrepChangeLoggedness(rel, true);
+ 			tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
+ 			/* force rewrite if necessary */
+ 			if (tab->chgLoggedness)
+ 				tab->rewrite = true;
+ 			pass = AT_PASS_MISC;
+ 			break;
+ 		case AT_SetUnLogged:	/* SET UNLOGGED */
+ 			ATSimplePermissions(rel, ATT_TABLE);
+ 			tab->chgLoggedness = ATPrepChangeLoggedness(rel, false);
+ 			tab->newrelpersistence = RELPERSISTENCE_UNLOGGED;
+ 			/* force rewrite if necessary */
+ 			if (tab->chgLoggedness)
+ 				tab->rewrite = true;
+ 			pass = AT_PASS_MISC;
+ 			break;
  		case AT_AddOids:		/* SET WITH OIDS */
  			ATSimplePermissions(rel, ATT_TABLE);
  			if (!rel->rd_rel->relhasoids || recursing)
***************
*** 3430,3435 **** ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
--- 3459,3467 ----
  		case AT_DropCluster:	/* SET WITHOUT CLUSTER */
  			ATExecDropCluster(rel, lockmode);
  			break;
+ 		case AT_SetLogged:		/* SET LOGGED */
+ 		case AT_SetUnLogged:	/* SET UNLOGGED */
+ 			break;
  		case AT_AddOids:		/* SET WITH OIDS */
  			/* Use the ADD COLUMN code, unless prep decided to do nothing */
  			if (cmd->def != NULL)
***************
*** 3583,3589 **** ATRewriteTables(List **wqueue, LOCKMODE lockmode)
  
  		/*
  		 * We only need to rewrite the table if at least one column needs to
! 		 * be recomputed, or we are adding/removing the OID column.
  		 */
  		if (tab->rewrite)
  		{
--- 3615,3622 ----
  
  		/*
  		 * We only need to rewrite the table if at least one column needs to
! 		 * be recomputed, we are adding/removing the OID column, or we are
! 		 * changing its persistence.
  		 */
  		if (tab->rewrite)
  		{
***************
*** 3591,3596 **** ATRewriteTables(List **wqueue, LOCKMODE lockmode)
--- 3624,3630 ----
  			Relation	OldHeap;
  			Oid			OIDNewHeap;
  			Oid			NewTableSpace;
+ 			char		persistence;
  
  			OldHeap = heap_open(tab->relid, NoLock);
  
***************
*** 3629,3638 **** ATRewriteTables(List **wqueue, LOCKMODE lockmode)
  			else
  				NewTableSpace = OldHeap->rd_rel->reltablespace;
  
  			heap_close(OldHeap, NoLock);
  
! 			/* Create transient table that will receive the modified data */
! 			OIDNewHeap = make_new_heap(tab->relid, NewTableSpace, false,
  									   lockmode);
  
  			/*
--- 3663,3693 ----
  			else
  				NewTableSpace = OldHeap->rd_rel->reltablespace;
  
+ 			/*
+ 			 * Select persistence of transient table (same as original unless
+ 			 * user requested a change)
+ 			 */
+ 			persistence = tab->chgLoggedness ?
+ 				tab->newrelpersistence : OldHeap->rd_rel->relpersistence;
+ 
  			heap_close(OldHeap, NoLock);
  
! 			/*
! 			 * Create transient table that will receive the modified data.
! 			 *
! 			 * Ensure it is marked correctly as logged or unlogged.  We have
! 			 * to do this here so that buffers for the new relfilenode will
! 			 * have the right persistence set, and at the same time ensure
! 			 * that the original filenode's buffers will get read in with the
! 			 * correct setting (i.e. the original one).  Otherwise a rollback
! 			 * after the rewrite would possibly result with buffers for the
! 			 * original filenode having the wrong persistence setting.
! 			 *
! 			 * NB: This relies on swap_relation_files() also swapping the
! 			 * persistence. That wouldn't work for pg_class, but that can't be
! 			 * unlogged anyway.
! 			 */
! 			OIDNewHeap = make_new_heap(tab->relid, NewTableSpace, persistence,
  									   lockmode);
  
  			/*
***************
*** 3643,3648 **** ATRewriteTables(List **wqueue, LOCKMODE lockmode)
--- 3698,3713 ----
  			ATRewriteTable(tab, OIDNewHeap, lockmode);
  
  			/*
+ 			 * Change the persistence marking of indexes, if necessary.  This
+ 			 * is so that the new copies are built with the right persistence
+ 			 * in the reindex step below.  Note we cannot do this earlier,
+ 			 * because the rewrite step might read the indexes, and that would
+ 			 * cause buffers for them to have the wrong setting.
+ 			 */
+ 			if (tab->chgLoggedness)
+ 				ATChangeIndexesLoggedness(tab->relid, tab->newrelpersistence);
+ 
+ 			/*
  			 * Swap the physical files of the old and new heaps, then rebuild
  			 * indexes and discard the old heap.  We can use RecentXmin for
  			 * the table's new relfrozenxid because we rewrote all the tuples
***************
*** 4052,4057 **** ATGetQueueEntry(List **wqueue, Relation rel)
--- 4117,4124 ----
  	tab->relid = relid;
  	tab->relkind = rel->rd_rel->relkind;
  	tab->oldDesc = CreateTupleDescCopy(RelationGetDescr(rel));
+ 	tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
+ 	tab->chgLoggedness = false;
  
  	*wqueue = lappend(*wqueue, tab);
  
***************
*** 10430,10435 **** ATExecGenericOptions(Relation rel, List *options)
--- 10497,10664 ----
  }
  
  /*
+  * Preparation phase for SET LOGGED/UNLOGGED
+  *
+  * This verifies that we're not trying to change a temp table.  Also,
+  * existing foreign key constraints are checked to avoid ending up with
+  * permanent tables referencing unlogged tables.
+  *
+  * Return value is false if the operation is a no-op (in which case the
+  * checks are skipped), otherwise true.
+  */
+ static bool
+ ATPrepChangeLoggedness(Relation rel, bool toLogged)
+ {
+ 	Relation	pg_constraint;
+ 	HeapTuple	tuple;
+ 	SysScanDesc scan;
+ 	ScanKeyData skey[1];
+ 
+ 	/*
+ 	 * Disallow changing status for a temp table.  Also verify whether we can
+ 	 * get away with doing nothing; in such cases we don't need to run the
+ 	 * checks below, either.
+ 	 */
+ 	switch (rel->rd_rel->relpersistence)
+ 	{
+ 		case RELPERSISTENCE_TEMP:
+ 			ereport(ERROR,
+ 					(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ 					 errmsg("cannot change logged status of table %s",
+ 							RelationGetRelationName(rel)),
+ 					 errdetail("Table %s is temporary.",
+ 							   RelationGetRelationName(rel)),
+ 					 errtable(rel)));
+ 			break;
+ 		case RELPERSISTENCE_PERMANENT:
+ 			if (toLogged)
+ 				/* nothing to do */
+ 				return false;
+ 			break;
+ 		case RELPERSISTENCE_UNLOGGED:
+ 			if (!toLogged)
+ 				/* nothing to do */
+ 				return false;
+ 			break;
+ 	}
+ 
+ 	/*
+ 	 * Check existing foreign key constraints to preserve the invariant that
+ 	 * no permanent tables cannot reference unlogged ones.  Self-referencing
+ 	 * foreign keys can safely be ignored.
+ 	 */
+ 	pg_constraint = heap_open(ConstraintRelationId, AccessShareLock);
+ 
+ 	/*
+ 	 * Scan conrelid if changing to permanent, else confrelid.  This also
+ 	 * determines whether an useful index exists.
+ 	 */
+ 	ScanKeyInit(&skey[0],
+ 				toLogged ? Anum_pg_constraint_conrelid :
+ 				Anum_pg_constraint_confrelid,
+ 				BTEqualStrategyNumber, F_OIDEQ,
+ 				ObjectIdGetDatum(RelationGetRelid(rel)));
+ 	scan = systable_beginscan(pg_constraint,
+ 							  toLogged ? ConstraintRelidIndexId : InvalidOid,
+ 							  true, NULL, 1, skey);
+ 
+ 	while (HeapTupleIsValid(tuple = systable_getnext(scan)))
+ 	{
+ 		Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tuple);
+ 
+ 		if (con->contype == CONSTRAINT_FOREIGN)
+ 		{
+ 			Oid			foreignrelid;
+ 			Relation	foreignrel;
+ 
+ 			/* the opposite end of what we used as scankey */
+ 			foreignrelid = toLogged ? con->confrelid : con->conrelid;
+ 
+ 			/* ignore if self-referencing */
+ 			if (RelationGetRelid(rel) == foreignrelid)
+ 				continue;
+ 
+ 			foreignrel = relation_open(foreignrelid, AccessShareLock);
+ 
+ 			if (toLogged)
+ 			{
+ 				if (foreignrel->rd_rel->relpersistence != RELPERSISTENCE_PERMANENT)
+ 					ereport(ERROR,
+ 							(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ 						 errmsg("cannot change status of table %s to logged",
+ 								RelationGetRelationName(rel)),
+ 						  errdetail("Table %s references unlogged table %s.",
+ 									RelationGetRelationName(rel),
+ 									RelationGetRelationName(foreignrel)),
+ 							 errtableconstraint(rel, NameStr(con->conname))));
+ 			}
+ 			else
+ 			{
+ 				if (foreignrel->rd_rel->relpersistence == RELPERSISTENCE_PERMANENT)
+ 					ereport(ERROR,
+ 							(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ 					   errmsg("cannot change status of table %s to unlogged",
+ 							  RelationGetRelationName(rel)),
+ 					  errdetail("Logged table %s is referenced by table %s.",
+ 								RelationGetRelationName(foreignrel),
+ 								RelationGetRelationName(rel)),
+ 							 errtableconstraint(rel, NameStr(con->conname))));
+ 			}
+ 
+ 			relation_close(foreignrel, AccessShareLock);
+ 		}
+ 	}
+ 
+ 	systable_endscan(scan);
+ 
+ 	heap_close(pg_constraint, AccessShareLock);
+ 
+ 	return true;
+ }
+ 
+ /*
+  * Update the pg_class entry of each index for the given relation to the
+  * given persistence.
+  */
+ static void
+ ATChangeIndexesLoggedness(Oid relid, char relpersistence)
+ {
+ 	Relation	rel;
+ 	Relation	pg_class;
+ 	List	   *indexes;
+ 	ListCell   *cell;
+ 
+ 	pg_class = heap_open(RelationRelationId, RowExclusiveLock);
+ 
+ 	/* We already have a lock on the table */
+ 	rel = relation_open(relid, NoLock);
+ 	indexes = RelationGetIndexList(rel);
+ 	foreach(cell, indexes)
+ 	{
+ 		Oid			indexid = lfirst_oid(cell);
+ 		HeapTuple	tuple;
+ 		Form_pg_class pg_class_form;
+ 
+ 		tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexid));
+ 		if (!HeapTupleIsValid(tuple))
+ 			elog(ERROR, "cache lookup failed for relation %u",
+ 				 indexid);
+ 
+ 		pg_class_form = (Form_pg_class) GETSTRUCT(tuple);
+ 		pg_class_form->relpersistence = relpersistence;
+ 		simple_heap_update(pg_class, &tuple->t_self, tuple);
+ 
+ 		/* keep catalog indexes current */
+ 		CatalogUpdateIndexes(pg_class, tuple);
+ 
+ 		heap_freetuple(tuple);
+ 	}
+ 
+ 	heap_close(pg_class, RowExclusiveLock);
+ 	heap_close(rel, NoLock);
+ }
+ 
+ /*
   * Execute ALTER TABLE SET SCHEMA
   */
  Oid
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 577,583 **** static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
  
  	LABEL LANGUAGE LARGE_P LAST_P LATERAL_P
  	LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
! 	LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P
  
  	MAPPING MATCH MATERIALIZED MAXVALUE MINUTE_P MINVALUE MODE MONTH_P MOVE
  
--- 577,583 ----
  
  	LABEL LANGUAGE LARGE_P LAST_P LATERAL_P
  	LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
! 	LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P LOGGED
  
  	MAPPING MATCH MATERIALIZED MAXVALUE MINUTE_P MINVALUE MODE MONTH_P MOVE
  
***************
*** 2048,2053 **** alter_table_cmd:
--- 2048,2067 ----
  					n->name = NULL;
  					$$ = (Node *)n;
  				}
+ 			/* ALTER TABLE <name> SET LOGGED  */
+ 			| SET LOGGED
+ 				{
+ 					AlterTableCmd *n = makeNode(AlterTableCmd);
+ 					n->subtype = AT_SetLogged;
+ 					$$ = (Node *)n;
+ 				}
+ 			/* ALTER TABLE <name> SET UNLOGGED  */
+ 			| SET UNLOGGED
+ 				{
+ 					AlterTableCmd *n = makeNode(AlterTableCmd);
+ 					n->subtype = AT_SetUnLogged;
+ 					$$ = (Node *)n;
+ 				}
  			/* ALTER TABLE <name> ENABLE TRIGGER <trig> */
  			| ENABLE_P TRIGGER name
  				{
***************
*** 12992,12997 **** unreserved_keyword:
--- 13006,13012 ----
  			| LOCAL
  			| LOCATION
  			| LOCK_P
+ 			| LOGGED
  			| MAPPING
  			| MATCH
  			| MATERIALIZED
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***************
*** 1641,1652 **** psql_completion(const char *text, int start, int end)
  		completion_info_charp = prev3_wd;
  		COMPLETE_WITH_QUERY(Query_for_index_of_table);
  	}
! 	/* If we have TABLE <sth> SET, provide WITHOUT,TABLESPACE and SCHEMA */
  	else if (pg_strcasecmp(prev3_wd, "TABLE") == 0 &&
  			 pg_strcasecmp(prev_wd, "SET") == 0)
  	{
  		static const char *const list_TABLESET[] =
! 		{"(", "WITHOUT", "TABLESPACE", "SCHEMA", NULL};
  
  		COMPLETE_WITH_LIST(list_TABLESET);
  	}
--- 1641,1652 ----
  		completion_info_charp = prev3_wd;
  		COMPLETE_WITH_QUERY(Query_for_index_of_table);
  	}
! 	/* If we have TABLE <sth> SET, provide list of attributes and '(' */
  	else if (pg_strcasecmp(prev3_wd, "TABLE") == 0 &&
  			 pg_strcasecmp(prev_wd, "SET") == 0)
  	{
  		static const char *const list_TABLESET[] =
! 		{"(", "LOGGED", "SCHEMA", "TABLESPACE", "UNLOGGED", "WITH", "WITHOUT", NULL};
  
  		COMPLETE_WITH_LIST(list_TABLESET);
  	}
*** a/src/include/commands/cluster.h
--- b/src/include/commands/cluster.h
***************
*** 25,31 **** extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid,
  						   bool recheck, LOCKMODE lockmode);
  extern void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal);
  
! extern Oid make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, bool forcetemp,
  			  LOCKMODE lockmode);
  extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
  				 bool is_system_catalog,
--- 25,31 ----
  						   bool recheck, LOCKMODE lockmode);
  extern void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal);
  
! extern Oid make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, char relpersistence,
  			  LOCKMODE lockmode);
  extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
  				 bool is_system_catalog,
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
***************
*** 1307,1312 **** typedef enum AlterTableType
--- 1307,1314 ----
  	AT_ChangeOwner,				/* change owner */
  	AT_ClusterOn,				/* CLUSTER ON */
  	AT_DropCluster,				/* SET WITHOUT CLUSTER */
+ 	AT_SetLogged,				/* SET LOGGED */
+ 	AT_SetUnLogged,				/* SET UNLOGGED */
  	AT_AddOids,					/* SET WITH OIDS */
  	AT_AddOidsRecurse,			/* internal to commands/tablecmds.c */
  	AT_DropOids,				/* SET WITHOUT OIDS */
*** a/src/include/parser/kwlist.h
--- b/src/include/parser/kwlist.h
***************
*** 230,235 **** PG_KEYWORD("localtime", LOCALTIME, RESERVED_KEYWORD)
--- 230,236 ----
  PG_KEYWORD("localtimestamp", LOCALTIMESTAMP, RESERVED_KEYWORD)
  PG_KEYWORD("location", LOCATION, UNRESERVED_KEYWORD)
  PG_KEYWORD("lock", LOCK_P, UNRESERVED_KEYWORD)
+ PG_KEYWORD("logged", LOGGED, UNRESERVED_KEYWORD)
  PG_KEYWORD("mapping", MAPPING, UNRESERVED_KEYWORD)
  PG_KEYWORD("match", MATCH, UNRESERVED_KEYWORD)
  PG_KEYWORD("materialized", MATERIALIZED, UNRESERVED_KEYWORD)
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
***************
*** 2426,2428 **** TRUNCATE old_system_table;
--- 2426,2517 ----
  ALTER TABLE old_system_table DROP CONSTRAINT new_system_table_pkey;
  ALTER TABLE old_system_table DROP COLUMN othercol;
  DROP TABLE old_system_table;
+ -- set logged
+ CREATE UNLOGGED TABLE unlogged1(f1 SERIAL PRIMARY KEY, f2 TEXT);
+ -- check relpersistence of an unlogged table
+ SELECT relname, relkind, relpersistence FROM pg_class WHERE relname ~ '^unlogged1'
+ UNION ALL
+ SELECT 'toast table', t.relkind, t.relpersistence FROM pg_class r JOIN pg_class t ON t.oid = r.reltoastrelid WHERE r.relname ~ '^unlogged1'
+ UNION ALL
+ SELECT 'toast index', ri.relkind, ri.relpersistence FROM pg_class r join pg_class t ON t.oid = r.reltoastrelid JOIN pg_index i ON i.indrelid = t.oid JOIN pg_class ri ON ri.oid = i.indexrelid WHERE r.relname ~ '^unlogged1'
+ ORDER BY relname;
+      relname      | relkind | relpersistence 
+ ------------------+---------+----------------
+  toast index      | i       | u
+  toast table      | t       | u
+  unlogged1        | r       | u
+  unlogged1_f1_seq | S       | p
+  unlogged1_pkey   | i       | u
+ (5 rows)
+ 
+ CREATE UNLOGGED TABLE unlogged2(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES unlogged1); -- foreign key
+ CREATE UNLOGGED TABLE unlogged3(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES unlogged3); -- self-referencing foreign key
+ ALTER TABLE unlogged3 SET LOGGED; -- skip self-referencing foreign key
+ ALTER TABLE unlogged2 SET LOGGED; -- fails because a foreign key to an unlogged table exists
+ ERROR:  cannot change status of table unlogged2 to logged
+ DETAIL:  Table unlogged2 references unlogged table unlogged1.
+ ALTER TABLE unlogged1 SET LOGGED;
+ -- check relpersistence of an unlogged table after changing to permament
+ SELECT relname, relkind, relpersistence FROM pg_class WHERE relname ~ '^unlogged1'
+ UNION ALL
+ SELECT 'toast table', t.relkind, t.relpersistence FROM pg_class r JOIN pg_class t ON t.oid = r.reltoastrelid WHERE r.relname ~ '^unlogged1'
+ UNION ALL
+ SELECT 'toast index', ri.relkind, ri.relpersistence FROM pg_class r join pg_class t ON t.oid = r.reltoastrelid JOIN pg_index i ON i.indrelid = t.oid JOIN pg_class ri ON ri.oid = i.indexrelid WHERE r.relname ~ '^unlogged1'
+ ORDER BY relname;
+      relname      | relkind | relpersistence 
+ ------------------+---------+----------------
+  toast index      | i       | p
+  toast table      | t       | p
+  unlogged1        | r       | p
+  unlogged1_f1_seq | S       | p
+  unlogged1_pkey   | i       | p
+ (5 rows)
+ 
+ DROP TABLE unlogged3;
+ DROP TABLE unlogged2;
+ DROP TABLE unlogged1;
+ -- set unlogged
+ CREATE TABLE logged1(f1 SERIAL PRIMARY KEY, f2 TEXT);
+ -- check relpersistence of a permanent table
+ SELECT relname, relkind, relpersistence FROM pg_class WHERE relname ~ '^logged1'
+ UNION ALL
+ SELECT 'toast table', t.relkind, t.relpersistence FROM pg_class r JOIN pg_class t ON t.oid = r.reltoastrelid WHERE r.relname ~ '^logged1'
+ UNION ALL
+ SELECT 'toast index', ri.relkind, ri.relpersistence FROM pg_class r join pg_class t ON t.oid = r.reltoastrelid JOIN pg_index i ON i.indrelid = t.oid JOIN pg_class ri ON ri.oid = i.indexrelid WHERE r.relname ~ '^logged1'
+ ORDER BY relname;
+     relname     | relkind | relpersistence 
+ ----------------+---------+----------------
+  logged1        | r       | p
+  logged1_f1_seq | S       | p
+  logged1_pkey   | i       | p
+  toast index    | i       | p
+  toast table    | t       | p
+ (5 rows)
+ 
+ CREATE TABLE logged2(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES logged1); -- foreign key
+ CREATE TABLE logged3(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES logged3); -- self-referencing foreign key
+ ALTER TABLE logged1 SET UNLOGGED; -- fails because a foreign key from a permanent table exists
+ ERROR:  cannot change status of table logged1 to unlogged
+ DETAIL:  Logged table logged2 is referenced by table logged1.
+ ALTER TABLE logged3 SET UNLOGGED; -- skip self-referencing foreign key
+ ALTER TABLE logged2 SET UNLOGGED;
+ ALTER TABLE logged1 SET UNLOGGED;
+ -- check relpersistence of a permanent table after changing to unlogged
+ SELECT relname, relkind, relpersistence FROM pg_class WHERE relname ~ '^logged1'
+ UNION ALL
+ SELECT 'toast table', t.relkind, t.relpersistence FROM pg_class r JOIN pg_class t ON t.oid = r.reltoastrelid WHERE r.relname ~ '^logged1'
+ UNION ALL
+ SELECT 'toast index', ri.relkind, ri.relpersistence FROM pg_class r join pg_class t ON t.oid = r.reltoastrelid JOIN pg_index i ON i.indrelid = t.oid JOIN pg_class ri ON ri.oid = i.indexrelid WHERE r.relname ~ '^logged1'
+ ORDER BY relname;
+     relname     | relkind | relpersistence 
+ ----------------+---------+----------------
+  logged1        | r       | u
+  logged1_f1_seq | S       | p
+  logged1_pkey   | i       | u
+  toast index    | i       | u
+  toast table    | t       | u
+ (5 rows)
+ 
+ DROP TABLE logged3;
+ DROP TABLE logged2;
+ DROP TABLE logged1;
*** a/src/test/regress/sql/alter_table.sql
--- b/src/test/regress/sql/alter_table.sql
***************
*** 1624,1626 **** TRUNCATE old_system_table;
--- 1624,1676 ----
  ALTER TABLE old_system_table DROP CONSTRAINT new_system_table_pkey;
  ALTER TABLE old_system_table DROP COLUMN othercol;
  DROP TABLE old_system_table;
+ 
+ -- set logged
+ CREATE UNLOGGED TABLE unlogged1(f1 SERIAL PRIMARY KEY, f2 TEXT);
+ -- check relpersistence of an unlogged table
+ SELECT relname, relkind, relpersistence FROM pg_class WHERE relname ~ '^unlogged1'
+ UNION ALL
+ SELECT 'toast table', t.relkind, t.relpersistence FROM pg_class r JOIN pg_class t ON t.oid = r.reltoastrelid WHERE r.relname ~ '^unlogged1'
+ UNION ALL
+ SELECT 'toast index', ri.relkind, ri.relpersistence FROM pg_class r join pg_class t ON t.oid = r.reltoastrelid JOIN pg_index i ON i.indrelid = t.oid JOIN pg_class ri ON ri.oid = i.indexrelid WHERE r.relname ~ '^unlogged1'
+ ORDER BY relname;
+ CREATE UNLOGGED TABLE unlogged2(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES unlogged1); -- foreign key
+ CREATE UNLOGGED TABLE unlogged3(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES unlogged3); -- self-referencing foreign key
+ ALTER TABLE unlogged3 SET LOGGED; -- skip self-referencing foreign key
+ ALTER TABLE unlogged2 SET LOGGED; -- fails because a foreign key to an unlogged table exists
+ ALTER TABLE unlogged1 SET LOGGED;
+ -- check relpersistence of an unlogged table after changing to permament
+ SELECT relname, relkind, relpersistence FROM pg_class WHERE relname ~ '^unlogged1'
+ UNION ALL
+ SELECT 'toast table', t.relkind, t.relpersistence FROM pg_class r JOIN pg_class t ON t.oid = r.reltoastrelid WHERE r.relname ~ '^unlogged1'
+ UNION ALL
+ SELECT 'toast index', ri.relkind, ri.relpersistence FROM pg_class r join pg_class t ON t.oid = r.reltoastrelid JOIN pg_index i ON i.indrelid = t.oid JOIN pg_class ri ON ri.oid = i.indexrelid WHERE r.relname ~ '^unlogged1'
+ ORDER BY relname;
+ DROP TABLE unlogged3;
+ DROP TABLE unlogged2;
+ DROP TABLE unlogged1;
+ -- set unlogged
+ CREATE TABLE logged1(f1 SERIAL PRIMARY KEY, f2 TEXT);
+ -- check relpersistence of a permanent table
+ SELECT relname, relkind, relpersistence FROM pg_class WHERE relname ~ '^logged1'
+ UNION ALL
+ SELECT 'toast table', t.relkind, t.relpersistence FROM pg_class r JOIN pg_class t ON t.oid = r.reltoastrelid WHERE r.relname ~ '^logged1'
+ UNION ALL
+ SELECT 'toast index', ri.relkind, ri.relpersistence FROM pg_class r join pg_class t ON t.oid = r.reltoastrelid JOIN pg_index i ON i.indrelid = t.oid JOIN pg_class ri ON ri.oid = i.indexrelid WHERE r.relname ~ '^logged1'
+ ORDER BY relname;
+ CREATE TABLE logged2(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES logged1); -- foreign key
+ CREATE TABLE logged3(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES logged3); -- self-referencing foreign key
+ ALTER TABLE logged1 SET UNLOGGED; -- fails because a foreign key from a permanent table exists
+ ALTER TABLE logged3 SET UNLOGGED; -- skip self-referencing foreign key
+ ALTER TABLE logged2 SET UNLOGGED;
+ ALTER TABLE logged1 SET UNLOGGED;
+ -- check relpersistence of a permanent table after changing to unlogged
+ SELECT relname, relkind, relpersistence FROM pg_class WHERE relname ~ '^logged1'
+ UNION ALL
+ SELECT 'toast table', t.relkind, t.relpersistence FROM pg_class r JOIN pg_class t ON t.oid = r.reltoastrelid WHERE r.relname ~ '^logged1'
+ UNION ALL
+ SELECT 'toast index', ri.relkind, ri.relpersistence FROM pg_class r join pg_class t ON t.oid = r.reltoastrelid JOIN pg_index i ON i.indrelid = t.oid JOIN pg_class ri ON ri.oid = i.indexrelid WHERE r.relname ~ '^logged1'
+ ORDER BY relname;
+ DROP TABLE logged3;
+ DROP TABLE logged2;
+ DROP TABLE logged1;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to