> On Mar 24, 2026, at 23:35, Jim Jones <[email protected]> wrote:
> 
> Hi,
> 
> While testing another patch [1], I noticed that REPACK is blocked when a
> temporary table is locked in another session. It also turns out that the
> same behaviour occurs with VACUUM FULL and CLUSTER:
> 
> == session 1 ==
> 
> $ psql postgres
> psql (19devel)
> Type "help" for help.
> 
> postgres=# CREATE TEMPORARY TABLE tmp (id int);
> CREATE TABLE
> postgres=# BEGIN;
> LOCK TABLE tmp IN SHARE MODE;
> BEGIN
> LOCK TABLE
> postgres=*#
> 
> == session 2 ==
> 
> $ psql postgres
> psql (19devel)
> Type "help" for help.
> 
> postgres=# REPACK;
> ^CCancel request sent
> ERROR:  canceling statement due to user request
> CONTEXT:  waiting for AccessExclusiveLock on relation 38458 of database 5
> postgres=# VACUUM FULL;
> ^CCancel request sent
> ERROR:  canceling statement due to user request
> CONTEXT:  waiting for AccessExclusiveLock on relation 38458 of database 5
> 
> Skipping temporary relations in get_tables_to_repack() and
> get_all_vacuum_rels() before they're appended to the list seems to do
> the trick -- see attached draft.
> 
> I can reproduce the same behaviour with CLUSTER and VACUUM FULL in
> PG14-PG18. I took a quick look at the code in PG17 and PG18 and the fix
> appears to be straightforward, but before I start working on it, I'd
> like to hear your thoughts. Is it worth the effort?
> 
> Best, Jim
> 
> 1 - 
> https://www.postgresql.org/message-id/13637.1774342137%40localhost<v1-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch>

I think skipping temp tables of another session is reasonable, because anyway 
they are not accessible from the current session, though visible via pg_class.

Looking at the patch:
```
+                       /* Skip temp relations belonging to other sessions */
+                       if (class->relpersistence == RELPERSISTENCE_TEMP &&
+                               isOtherTempNamespace(class->relnamespace))
```

It uses isOtherTempNamespace(), but I noticed that the header comment of the 
function says:
```
 * isOtherTempNamespace - is the given namespace some other backend's
* temporary-table namespace (including temporary-toast-table namespaces)?
*
* Note: for most purposes in the C code, this function is obsolete. Use
* RELATION_IS_OTHER_TEMP() instead to detect non-local temp relations.
```

Then looking at RELATION_IS_OTHER_TEMP():
```
#define RELATION_IS_OTHER_TEMP(relation) \
        ((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP && \
         !(relation)->rd_islocaltemp)
```

It takes a relation as parameter and check relation->rd_islocaltemp, however in 
the context of this patch, we have only Form_pg_class.

Then checking how rd_islocaltemp is set:
```
                case RELPERSISTENCE_TEMP:
                        if 
(isTempOrTempToastNamespace(relation->rd_rel->relnamespace))
                        {
                                relation->rd_backend = 
ProcNumberForTempRelations();
                                relation->rd_islocaltemp = true;
                        }
                        else
                        {
                                /*
                                 * If it's a temp table, but not one of ours, 
we have to use
                                 * the slow, grotty method to figure out the 
owning backend.
                                 *
                                 * Note: it's possible that rd_backend gets set 
to
                                 * MyProcNumber here, in case we are looking at 
a pg_class
                                 * entry left over from a crashed backend that 
coincidentally
                                 * had the same ProcNumber we're using.  We 
should *not*
                                 * consider such a table to be "ours"; this is 
why we need the
                                 * separate rd_islocaltemp flag.  The pg_class 
entry will get
                                 * flushed if/when we clean out the 
corresponding temp table
                                 * namespace in preparation for using it.
                                 */
                                relation->rd_backend =
                                        
GetTempNamespaceProcNumber(relation->rd_rel->relnamespace);
                                Assert(relation->rd_backend != 
INVALID_PROC_NUMBER);
                                relation->rd_islocaltemp = false;
                        }
                        break;
```

It uses isTempOrTempToastNamespace(relation->rd_rel->relnamespace) to decide 
relation->rd_islocaltemp.

So, I think this patch should also use 
"!isTempOrTempToastNamespace(classForm->relnamespace)" instead of 
isOtherTempNamespace(class->relnamespace). I tried that locally, and it works 
for me.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/






Reply via email to