2015-12-01 17:52 GMT+01:00 Catalin Iacob <iacobcata...@gmail.com>: > On Tue, Dec 1, 2015 at 1:53 PM, Michael Paquier > <michael.paqu...@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. I have not strong option about this. Regards Pavel
