Horton

Re: Some codes refer slot()->{'slot_name'} but it is not defined

Fujii Masao <masao.fujii@oss.nttdata.com>

View thread View on postgr.es
From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, "'pgsql-hackers@lists.postgresql.org'" <pgsql-hackers@lists.postgresql.org>
Date: 2025-04-03T09:21:44Z
Lists: pgsql-hackers 

On 2025/04/03 12:15, Hayato Kuroda (Fujitsu) wrote:
> Dear hackers,
> 
> Cluster.pm defines a function slot()which requires a slot_name as a key
> and returns attributes of the given slot, as a hash-ref. ISTM, the hash
> does not contain 'slot_name'.
> 
> However, I found that some codes access it by using a key 'slot_name'. ISTM it always
> becomes 'undef' thus any tests are meaningless.
> 
> It looks like that existing codes want to check the existing of given logical slots.
> So, it is enough to search with key 'plugin'. The valid value is set if exists, otherwise ''.
> 
> How do you think?

I think you're right. The patch looks good to me.

-is($node_primary->slot('dropme_slot')->{'slot_name'},
-	undef, 'logical slot was actually dropped on standby');
+is($node_primary->slot('dropme_slot')->{'plugin'},
+	'', 'logical slot was actually dropped on standby');

This seems like a separate issue from what your patch is addressing,
but since this test is meant to confirm that the slot was dropped
on the standby, shouldn't node_primary be node_replica instead?

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

Commits

  1. Fix logical decoding test to correctly check slot removal on standby.

  2. Fix logical decoding regression tests to correctly check slot existence.