Re: [PATCH v4] Add \warn to psql

David Fetter <david(at)fetter(dot)org> writes:
> [ v7-0001-Add-warn-to-psql.patch ]

I took a look at this. I have no quibble with the proposed feature,
and the implementation is certainly simple enough. But I'm unconvinced
about the proposed test scaffolding. Spinning up a new PG instance is a
*hell* of a lot of overhead to pay for testing something that could be
tested as per attached. Admittedly, the attached doesn't positively
prove which pipe each output string went down, but that does not strike
me as a concern large enough to justify adding a TAP test for.

I'd be happier about adding TAP infrastructure if it looked like it'd
be usable to test some of the psql areas that are unreachable by the
existing test methodology, particularly tab-complete.c and prompt.c.
But I don't see anything here that looks like it'll work for that.

I don't like what you did to command_checks_all, either --- it could
hardly say "bolted on after the fact" more clearly if you'd written
that in <blink><red> text. If we need an input-stream argument,
let's just add it in a rational place and adjust the callers.
There aren't that many of 'em, nor has the subroutine been around
all that long.

regards, tom lane