Message-ID: <1b59d63b-8e89-17f7-685e-24ae23d04924@gmail.com>
Date: Thu, 27 Apr 2023 10:46:34 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.10.1
Subject: Re: Fix a test case in 035_standby_logical_decoding.pl
Content-Language: en-US
To: "Yu Shi (Fujitsu)" <shiy.fnst@fujitsu.com>,
 "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
References: <OSZPR01MB6310B0A507A0F2A2D379F38CFD6A9@OSZPR01MB6310.jpnprd01.prod.outlook.com>
From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
In-Reply-To: <OSZPR01MB6310B0A507A0F2A2D379F38CFD6A9@OSZPR01MB6310.jpnprd01.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/1b59d63b-8e89-17f7-685e-24ae23d04924%40gmail.com>
Precedence: bulk

Hi,

On 4/27/23 10:11 AM, Yu Shi (Fujitsu) wrote:
> Hi hackers,
> 
> In 035_standby_logical_decoding.pl, I think that the check in the following test
> case should be performed on the standby node, instead of the primary node, as
> the slot is created on the standby node.

Oh right, the current test is not done on the right node, thanks!

> The test currently passes because it
> only checks the return value of psql. It might be more appropriate to check the
> error message. 

Agree, and it's consistent with what is being done in 006_logical_decoding.pl.

> Please see the attached patch.
> 

+
+($result, $stdout, $stderr) = $node_standby->psql(
          'otherdb',
          "SELECT lsn FROM pg_logical_slot_peek_changes('behaves_ok_activeslot', NULL, NULL) ORDER BY lsn DESC LIMIT 1;"
-    ),
-    3,
-    'replaying logical slot from another database fails');
+    );
+ok( $stderr =~
+         m/replication slot "behaves_ok_activeslot" was not created in this database/,
+       "replaying logical slot from another database fails");


That does look good to me.

Nit: I wonder if while at it (as it was already there) we could not remove the " ORDER BY lsn DESC LIMIT 1" part of it.
It does not change anything regarding the test but looks more appropriate to me.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com