Re: Logical Replication and Character encoding

From: Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>
To: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, peter(dot)eisentraut(at)2ndquadrant(dot)com
Cc: noriyoshi(dot)shinoda(at)hpe(dot)com, pgsql-hackers(at)postgresql(dot)org, craig(at)2ndquadrant(dot)com
Subject: Re: Logical Replication and Character encoding
Date: 2017-03-06 10:13:48
Message-ID: a7c0cc59-9c21-238c-b6a7-d6eb519ad9ee@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 06/03/17 11:06, Kyotaro HORIGUCHI wrote:
> At Fri, 3 Mar 2017 21:31:24 -0500, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote in <88397afa-a8ec-8d8a-1c94-94a4795a3870(at)2ndquadrant(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Petr Jelinek 2017-03-06 10:27:43 Re: Logical replication existing data copy
Previous Message Kyotaro HORIGUCHI 2017-03-06 10:06:34 Re: Logical Replication and Character encoding