> On Mar 26, 2026, at 07:00, Jim Jones <[email protected]> wrote:
>
> Hi
>
> On 25/03/2026 21:38, Zsolt Parragi wrote:
>> Shouldn't the patch also include a tap test to verify that the change
>> works / fails without it?
>
> Definitely. I just didn't want to invest much time on tests before
> getting feedback on the issue itself.
>
>> + /* Skip temp relations belonging to other sessions */
>> + {
>> + Oid nsp = get_rel_namespace(index->indrelid);
>> +
>> + if (!isTempOrTempToastNamespace(nsp) && isAnyTempNamespace(nsp))
>> + {
>>
>> Doesn't this result in several repeated syscache lookups?
>>
>> There's already a SearchSysCacheExsists1 directly above this, then a
>> get_rel_namespace, then an isAnyTempNamespace. While this probably
>> isn't performance critical, this should be doable with a single
>> SearchSysCache1(RELOID...) and then a few conditions, similarly to the
>> else branch below this?
>
> You're right. Although it is not performance critical we can solve it
> with a single SearchSysCache1.
>
> PFA v3 with the improved fix (0001) and tests (0002).
>
> Thanks for the review!
>
> Best,
> Jim<v3-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch><v3-0002-Test-VACUUM-FULL-CLUSTER-and-REPACK-with-locked-t.patch>
I don't think such a TAP test is necessary.
One reason is that, if we look at the check right above the new one:
```
/*
* We include partitioned tables here; depending on which
operation is
* to be performed, caller will decide whether to process or
ignore
* them.
*/
if (classForm->relkind != RELKIND_RELATION &&
classForm->relkind != RELKIND_MATVIEW &&
classForm->relkind != RELKIND_PARTITIONED_TABLE)
continue;
```
I don't see a test specifically for that check either. So I don't think we need
a test for every individual path.
Second, based on [1] and [2], I got the impression that adding new tests is not
always welcome considering overall test runtime. Anyway, maybe I’m wrong, let
the committers judge that.
[1]
https://postgr.es/m/mtkrkkcn2tlhytumitpch5ubxiprv2jzvprf5r5m3mjeczvq4q@p6wkzbfxuyv2
<https://postgr.es/m/>
[2] https://postgr.es/m/[email protected]
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
