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