Tom Lane wrote:

> > I examined cluster.c and it does seem to be missing a check too.  I'm
> > not sure where to add one though; the best choice would be the place
> > where the list of rels is built, but that scans only pg_index, so it
> > doesn't have access to the namespace of each rel.  So one idea would be
> > to get the pg_class row for each candidate, but that seems slow.
> > Another idea would be to just add all the candidates and silently skip
> > the temp indexes in cluster_rel.
> 
> Yeah, an extra fetch of the pg_class row doesn't seem all that nice.
> I think you'd want to check it in approximately the same two places
> where pg_class_ownercheck() is applied (one for the 1-xact and one for
> the multi-xact path).

Actually, the 1-xact path does not need it, because the check is already
elsewhere.  We only need logic enough to skip temp tables silently while
building the list in the multi-xact path.  What this means is that very
few people, if any, clusters temp tables; because as soon as you do, a
plain CLUSTER command in another session errors out with

alvherre=# cluster;
ERROR:  cannot cluster temporary tables of other sessions

So, patch attached.

> Are there any other commands to be worried about?  I can't see any
> besides VACUUM/ANALYZE, and those seem covered.

I can't think of any.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Index: src/backend/commands/cluster.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/cluster.c,v
retrieving revision 1.162
diff -c -p -r1.162 cluster.c
*** src/backend/commands/cluster.c	19 May 2007 01:02:34 -0000	1.162
--- src/backend/commands/cluster.c	29 Aug 2007 22:07:04 -0000
*************** cluster_rel(RelToCluster *rvtc, bool rec
*** 276,281 ****
--- 276,287 ----
  		}
  
  		/*
+ 		 * Note: we don't need to check whether the table is a temp for a
+ 		 * remote session here, because it will be checked in
+ 		 * check_index_is_clusterable, below.
+ 		 */
+ 
+ 		/*
  		 * Check that the index still exists
  		 */
  		if (!SearchSysCacheExists(RELOID,
*************** swap_relation_files(Oid r1, Oid r2, Tran
*** 995,1004 ****
  }
  
  /*
!  * Get a list of tables that the current user owns and
!  * have indisclustered set.  Return the list in a List * of rvsToCluster
!  * with the tableOid and the indexOid on which the table is already
!  * clustered.
   */
  static List *
  get_tables_to_cluster(MemoryContext cluster_context)
--- 1001,1037 ----
  }
  
  /*
!  * Returns whether a pg_class tuple belongs to a temp namespace which is not
!  * our backend's.
!  */
! static bool
! relid_is_other_temp(Oid class_oid)
! {
! 	HeapTuple	tuple;
! 	Form_pg_class classForm;
! 	bool		istemp;
! 
! 	tuple = SearchSysCache(RELOID,
! 						   ObjectIdGetDatum(class_oid),
! 						   0, 0, 0);
! 	if (!HeapTupleIsValid(tuple))
! 		ereport(ERROR,
! 				(errcode(ERRCODE_UNDEFINED_TABLE),
! 				 errmsg("relation with OID %u does not exist", class_oid)));
! 
! 	classForm = (Form_pg_class) GETSTRUCT(tuple);
! 	istemp = isOtherTempNamespace(classForm->relnamespace);
! 
! 	ReleaseSysCache(tuple);
! 
! 	return istemp;
! }
! 
! /*
!  * Get a list of tables that the current user owns, have indisclustered set,
!  * and are not temp tables of remote backends.  Return the list in a List * of
!  * rvsToCluster with the tableOid and the indexOid on which the table is
!  * already clustered.
   */
  static List *
  get_tables_to_cluster(MemoryContext cluster_context)
*************** get_tables_to_cluster(MemoryContext clus
*** 1031,1036 ****
--- 1064,1072 ----
  		if (!pg_class_ownercheck(index->indrelid, GetUserId()))
  			continue;
  
+ 		if (relid_is_other_temp(index->indrelid))
+ 			continue;
+ 
  		/*
  		 * We have to build the list in a different memory context so it will
  		 * survive the cross-transaction processing
Index: src/backend/commands/indexcmds.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/indexcmds.c,v
retrieving revision 1.162
diff -c -p -r1.162 indexcmds.c
*** src/backend/commands/indexcmds.c	25 Aug 2007 19:08:19 -0000	1.162
--- src/backend/commands/indexcmds.c	25 Aug 2007 22:11:18 -0000
*************** ReindexDatabase(const char *databaseName
*** 1292,1297 ****
--- 1292,1301 ----
  		if (classtuple->relkind != RELKIND_RELATION)
  			continue;
  
+ 		/* Skip temp tables of other backends; we can't reindex them at all */
+ 		if (isOtherTempNamespace(classtuple->relnamespace))
+ 			continue;
+ 
  		/* Check user/system classification, and optionally skip */
  		if (IsSystemClass(classtuple))
  		{
---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
       choose an index scan if your joining column's datatypes do not
       match

Reply via email to