On 06/03/17 11:06, Kyotaro HORIGUCHI wrote: > At Fri, 3 Mar 2017 21:31:24 -0500, Peter Eisentraut > <peter.eisentr...@2ndquadrant.com> wrote in > <88397afa-a8ec-8d8a-1c94-94a4795a3...@2ndquadrant.com> >> On 3/3/17 14:51, Petr Jelinek wrote: >>> On 03/03/17 20:37, Peter Eisentraut wrote: >>>> On 2/27/17 00:23, Kyotaro HORIGUCHI wrote: >>>>> Yeah, the patch sends converted string with the length of the >>>>> orignal length. Usually encoding conversion changes the length of >>>>> a string. I doubt that the reverse case was working correctly. >>>> >>>> I think we shouldn't send the length value at all. This might have been >>>> a leftover from an earlier version of the patch. >>>> >>>> See attached patch that removes the length value. >>>> >>> >>> Well the length is necessary to be able to add binary format support in >>> future so it's definitely not an omission. >> >> Right. So it appears the right function to use here is >> pq_sendcountedtext(). However, this sends the strings without null >> termination, so we'd have to do extra processing on the receiving end. >> Or we could add an option to pq_sendcountedtext() to make it do what we >> want. I'd rather stick to standard pqformat.c functions for handling >> the protocol. > > It seems reasonable. I changed the prototype of > pg_sendcountedtext as the following, > > | extern void pq_sendcountedtext(StringInfo buf, const char *str, int slen, > | bool countincludesself, bool binary); > > I think 'binary' seems fine for the parameter here. The patch > size increased a bit. >
Hmm I find it bit weird that binary true means NULL terminated while false means not NULL terminated, I would think that the behaviour would be exactly opposite given that text strings are the ones where NULL termination matters? > By the way, I noticed that postmaster launches logical > replication launcher even if wal_level < logical. Is it right? Yes, that came up couple of times in various threads. The logical replication launcher is needed only to start subscriptions and subscriptions don't need wal_level to be logical, only publication needs that. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
