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

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

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

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