On Tue, Dec 1, 2015 at 1:53 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > Attached is a patch implementing those suggestions. This simplifies > the code without changing its usefulness. If you are fine with those > changes I will switch this patch as ready for committer.
I tested the v07 patch (so not Michael's version) a few days ago but didn't send this email earlier.
I combined various -c and -f with --echo-all, --single-transaction, \set ON_ERROR_STOP=1, separate -c "VACUUM", "SELECT + VACUUM" in a single and in two -c, inserting -f - somewhere in the middle of the other -c and -f. They all behave as I would expect.
One maybe slightly surprising behaviour is that -f - can be specified multiple times and only the first one has an effect since the others act on an exhausted stdin. But I don't think forbidding multiple -f - is better.
As for the code (these still apply to Michael's latest patch):
1. the be compiler quiete comment is not good English, /* silence the compiler */ would be better or remove it completely
2. shouldn't atyp in SimpleActionListCell be of type enum _atypes? Otherwise why an enum if it's casted to int when actually used? If it's an enum the repeated ifs on cell->atyp should be a switch, either with a default Assert(0) or no default which makes gcc give a warning if an enum value is ever added without having a corresponding case.
It is maybe different topic - the psql uses enums and ints very freely. So I wrote code consistent with current code.
The code there uses some older patterns and the cleaning should be bigger patch.