On Wed, Aug 28, 2019 at 12:19:08PM -0400, Tom Lane wrote:
> 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 am fine with that approach. There is an argument for dropping safe_psql then? > 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? I got the same thought as you on this point about why the is() is actually necessary if we know that it would fail. An advantage of the current code is that we are able to catch all errors in a given run at once. If the test dies immediately, you cannot catch that and this needs repetitive retries if there is no dependency between each step of the test. An argument against back-patching the stuff changing the default flag are tests which rely on the current behavior. This could be annoying for some people doing advanced testing. -- Michael
signature.asc
Description: PGP signature
