Re: libpq changes for synchronous replication
Greg Smith <greg@2ndquadrant.com>
From: Greg Smith <greg@2ndquadrant.com>
To: Alvaro Herrera <alvherre@commandprompt.com>
Cc: Fujii Masao <masao.fujii@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>, Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2010-12-05T18:07:33Z
Lists: pgsql-hackers
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. Alvaro Herrera wrote: > Excerpts from Fujii Masao's message of jue nov 25 10:47:12 -0300 2010: > > >> The attached patch s/CopyXLog/CopyBoth/g and adds the description >> about CopyBoth into the COPY section. >> > > I gave this a look. It seems good, but I'm not sure about this bit: > > + case 'W': /* Start Copy Both */ > + /* > + * We don't need to use getCopyStart here since CopyBothResponse > + * specifies neither the copy format nor the number of columns in > + * the Copy data. They should be always zero. > + */ > + conn->result = PQmakeEmptyPGresult(conn, PGRES_COPY_OUT); > + if (!conn->result) > + return; > + conn->asyncStatus = PGASYNC_COPY_BOTH; > + conn->copy_already_done = 0; > + break; > > 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? > > (The paragraph added to the docs is also a bit too specific about this > being used exclusively in streaming replication, ISTM) > > >> 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. >