On Mon, Dec 6, 2010 at 12:54 AM, Fujii Masao <masao.fu...@gmail.com> wrote: > On Mon, Dec 6, 2010 at 3:07 AM, Greg Smith <g...@2ndquadrant.com> wrote: >> The one time this year top-posting seems appropriate...this patch seems >> stalled waiting for some sort of response to the concerns Alvaro raised >> here. > > Sorry for the delay. I didn't have the time. > >> I gave this a look. It seems good, but I'm not sure about this bit: > > Thanks for the review! > >> I guess this was OK when this was conceived as CopyXlog, but since it's >> now a generic mechanism, this seems a bit unwise. Should this be >> reconsidered so that it's possible to change the format or number of >> columns? > > I changed CopyBothResponse message so that it includes the format > and number of columns of copy data. Please see the attached patch. > >> (The paragraph added to the docs is also a bit too specific about this >> being used exclusively in streaming replication, ISTM) > > Yes. But it seems difficult to generalize the docs more because currently > only SR uses Copy-both. So I had to write that, for example, the condition > to get into the state is only "START REPLICATION" command. > >> While modifying the code, it occurred to me that we might have to add new >> ExecStatusType like PGRES_COPY_BOTH and use that for CopyBoth mode, >> for the sake of consistency. But since it's just alias of PGRES_COPY_BOTH >> for now, i.e., there is no specific behavior for that ExecStatusType, I >> don't >> think that it's worth adding that yet. >> >> >> I'm not so sure about this. If we think that it's worth adding a new >> possible state, we should do so now; we will not be able to change this >> behavior later. > > OK. I added that new state.
Committed with just a few changes to the documentation. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers