Thread: pgsql: Fix 007_sync_rep.pl to notice failures in ALTER SYSTEM SET.

pgsql: Fix 007_sync_rep.pl to notice failures in ALTER SYSTEM SET.

From
Tom Lane
Date:
Fix 007_sync_rep.pl to notice failures in ALTER SYSTEM SET.

If a test case tried to set an invalid value of synchronous_standby_names,
the test script didn't detect that, which seems like a bad idea.
Noticed while testing a proposed patch that broke some of these
test cases.

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/fb57f40eec503d637bf01c298f5cb2472f0d4fdb

Modified Files
--------------
src/test/recovery/t/007_sync_rep.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


Re: pgsql: Fix 007_sync_rep.pl to notice failures in ALTER SYSTEMSET.

From
Michael Paquier
Date:
On Mon, Aug 26, 2019 at 09:03:08PM +0000, Tom Lane wrote:
> Fix 007_sync_rep.pl to notice failures in ALTER SYSTEM SET.
>
> If a test case tried to set an invalid value of synchronous_standby_names,
> the test script didn't detect that, which seems like a bad idea.
> Noticed while testing a proposed patch that broke some of these
> test cases.

Just for the note.  I think that this needs a closer lookup and I am
afraid that there are more issues like this one.  For example, in
012_subtransactions.pl, pg_reload_conf() is called with
PostgresNode::psql without checking for an error which is a bad idea
after switching to synchonous replication.  Most of the transactions
done in this scrupt should also complain immediately on an error.
--
Michael

Attachment

Re: pgsql: Fix 007_sync_rep.pl to notice failures in ALTER SYSTEM SET.

From
Tom Lane
Date:
Michael Paquier <michael@paquier.xyz> writes:
> On Mon, Aug 26, 2019 at 09:03:08PM +0000, Tom Lane wrote:
>> Fix 007_sync_rep.pl to notice failures in ALTER SYSTEM SET.

> Just for the note.  I think that this needs a closer lookup and I am
> afraid that there are more issues like this one.

Yeah, it might be a good idea to make a sweep to see what other tests
should be using safe_psql.

Or, more radically, we could think about redefining PostgresNode::psql
to complain about errors, and you have to use something else if you
want it not to?

            regards, tom lane



Re: pgsql: Fix 007_sync_rep.pl to notice failures in ALTER SYSTEMSET.

From
Michael Paquier
Date:
On Tue, Aug 27, 2019 at 11:38:28PM -0400, Tom Lane wrote:
> Yeah, it might be a good idea to make a sweep to see what other tests
> should be using safe_psql.

I found more of these while browsing all the tests, so I am just going
to start a new thread with a patch.

> Or, more radically, we could think about redefining PostgresNode::psql
> to complain about errors, and you have to use something else if you
> want it not to?

Not sure about redesigning that as it has been around for a couple of
years now, and that would mean perhaps a third wrapper on top of
PostgresNode::psql.
--
Michael

Attachment