Re: Improve error detections in TAP tests by spreading safe_psql

I wrote:
> After a brief review of node->psql calls in the TAP tests, I'm
> of the opinion that what we should do is revert fb57f40 and instead
> change PostgresNode::psql so that on_error_die defaults to 1, then
> fix the *very* small number of callers that need it to be zero.
> Otherwise we're just going to be fighting this same fire forevermore.
> Moreover, that's going to lead to a much smaller patch.

Hmm .. I experimented with doing it this way, as attached, and I guess
I have to take back the assertion that it'd be a much smaller patch.
I found 47 calls that'd need to be changed to set on_error_die to 0,
whereas your patch is touching 44 calls (though I think it missed some).
I still think this is a safer, less bug-prone way to proceed though.

The attached patch just follows a mechanical rule of "set on_error_die
to 0 in exactly those calls where the surrounding code verifies the
exit code is what it expects". That leads to a lot of code that
looks like, say,

# Insert should work on standby
is( $node_standby->psql(
'postgres',
- qq{insert into testtab select generate_series(1,1000), 'foo';}),
+ qq{insert into testtab select generate_series(1,1000), 'foo';},
+ on_error_die => 0),
0,
'INSERT succeeds with truncated relation FSM');

I have to wonder if it isn't better to just drop the is() test
and let PostgresNode::psql issue the failure. The only thing
the is() is really buying us is the ability to label the test
with an ID string, which is helpful, but um ... couldn't that
just be a comment? Or we could think about adding another
optional parameter:

$node_standby->psql(
'postgres',
qq{insert into testtab select generate_series(1,1000), 'foo';},
test_name => 'INSERT succeeds with truncated relation FSM');

regards, tom lane