Logical Replication and Character encoding

Hello,

At Tue, 31 Jan 2017 12:46:18 +0000, "Shinoda, Noriyoshi" <noriyoshi.shinoda@hpe.com> wrote in <AT5PR84MB0084FAE5976D89CDE9733093EE4A0@AT5PR84MB0084.NAMPRD84.PROD.OUTLOOK.COM>

I tried a committed Logical Replication environment. I found
that replication between databases of different encodings did
not convert encodings in character type columns. Is this
behavior correct?

The output plugin for subscription is pgoutput and it currently
doesn't consider encoding but would easiliy be added if desired
encoding is informed.

The easiest (but somewhat seems fragile) way I can guess is,

- Subscriber connects with client_encoding specification and the
output plugin pgoutput decide whether it accepts the encoding
or not. If the subscriber doesn't, pgoutput send data without
conversion.

The attached small patch does this and works with the following
CREATE SUBSCRIPTION.

CREATE SUBSCRIPTION sub1 CONNECTION 'host=/tmp port=5432 dbname=postgres client_encoding=EUC_JP' PUBLICATION pub1;

Also we may have explicit negotiation on, for example,
CREATE_REPLICATION_SLOT.

'CREATE_REPLICATION_SLOT sub1 LOGICAL pgoutput ENCODING EUC_JP'

Or output plugin may take options.

'CREATE_REPLICATION_SLOT sub1 LOGICAL pgoutput OPTIONS(encoding EUC_JP)'

Any opinions?

--
Kyotaro Horiguchi
NTT Open Source Software Center

At Wed, 01 Feb 2017 12:05:40 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20170201.120540.183393194.horiguchi.kyotaro@lab.ntt.co.jp>

Hello,

At Tue, 31 Jan 2017 12:46:18 +0000, "Shinoda, Noriyoshi" <noriyoshi.shinoda@hpe.com> wrote in <AT5PR84MB0084FAE5976D89CDE9733093EE4A0@AT5PR84MB0084.NAMPRD84.PROD.OUTLOOK.COM>

I tried a committed Logical Replication environment. I found
that replication between databases of different encodings did
not convert encodings in character type columns. Is this
behavior correct?

The output plugin for subscription is pgoutput and it currently
doesn't consider encoding but would easiliy be added if desired
encoding is informed.

The easiest (but somewhat seems fragile) way I can guess is,

- Subscriber connects with client_encoding specification and the
output plugin pgoutput decide whether it accepts the encoding
or not. If the subscriber doesn't, pgoutput send data without
conversion.

The attached small patch does this and works with the following
CREATE SUBSCRIPTION.

Oops. It forgets to care conversion failure. It is amended in the
attached patch.

CREATE SUBSCRIPTION sub1 CONNECTION 'host=/tmp port=5432 dbname=postgres client_encoding=EUC_JP' PUBLICATION pub1;

Also we may have explicit negotiation on, for example,
CREATE_REPLICATION_SLOT.

'CREATE_REPLICATION_SLOT sub1 LOGICAL pgoutput ENCODING EUC_JP'

Or output plugin may take options.

'CREATE_REPLICATION_SLOT sub1 LOGICAL pgoutput OPTIONS(encoding EUC_JP)'

Any opinions?

--
Kyotaro Horiguchi
NTT Open Source Software Center

At Wed, 01 Feb 2017 12:13:04 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20170201.121304.267734380.horiguchi.kyotaro@lab.ntt.co.jp>

I tried a committed Logical Replication environment. I found
that replication between databases of different encodings did
not convert encodings in character type columns. Is this
behavior correct?

The output plugin for subscription is pgoutput and it currently
doesn't consider encoding but would easiliy be added if desired
encoding is informed.

The easiest (but somewhat seems fragile) way I can guess is,

- Subscriber connects with client_encoding specification and the
output plugin pgoutput decide whether it accepts the encoding
or not. If the subscriber doesn't, pgoutput send data without
conversion.

The attached small patch does this and works with the following
CREATE SUBSCRIPTION.

Oops. It forgets to care conversion failure. It is amended in the
attached patch.

CREATE SUBSCRIPTION sub1 CONNECTION 'host=/tmp port=5432 dbname=postgres client_encoding=EUC_JP' PUBLICATION pub1;

Also we may have explicit negotiation on, for example,
CREATE_REPLICATION_SLOT.

'CREATE_REPLICATION_SLOT sub1 LOGICAL pgoutput ENCODING EUC_JP'

Or output plugin may take options.

'CREATE_REPLICATION_SLOT sub1 LOGICAL pgoutput OPTIONS(encoding EUC_JP)'

Any opinions?

This patch chokes replication when the publisher finds an
inconvertible character in a tuple to be sent. For the case,
dropping-then-recreating subscription is necessary to go forward.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Thank you for creating patches.
I strongly hope that your patch will be merged into the new version.
Since all databases are not yet based on UTF - 8, I think conversion of character codes is still necessary.

-----Original Message-----
From: Kyotaro HORIGUCHI [mailto:horiguchi.kyotaro@lab.ntt.co.jp]
Sent: Wednesday, February 01, 2017 3:31 PM
To: Shinoda, Noriyoshi <noriyoshi.shinoda@hpe.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] Logical Replication and Character encoding

At Wed, 01 Feb 2017 12:13:04 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20170201.121304.267734380.horiguchi.kyotaro@lab.ntt.co.jp>

I tried a committed Logical Replication environment. I found
that replication between databases of different encodings did not
convert encodings in character type columns. Is this behavior
correct?

The output plugin for subscription is pgoutput and it currently
doesn't consider encoding but would easiliy be added if desired
encoding is informed.

The easiest (but somewhat seems fragile) way I can guess is,

- Subscriber connects with client_encoding specification and the
output plugin pgoutput decide whether it accepts the encoding
or not. If the subscriber doesn't, pgoutput send data without
conversion.

The attached small patch does this and works with the following
CREATE SUBSCRIPTION.

Oops. It forgets to care conversion failure. It is amended in the
attached patch.

CREATE SUBSCRIPTION sub1 CONNECTION 'host=/tmp port=5432
dbname=postgres client_encoding=EUC_JP' PUBLICATION pub1;

Also we may have explicit negotiation on, for example,
CREATE_REPLICATION_SLOT.

'CREATE_REPLICATION_SLOT sub1 LOGICAL pgoutput ENCODING EUC_JP'

Or output plugin may take options.

'CREATE_REPLICATION_SLOT sub1 LOGICAL pgoutput OPTIONS(encoding EUC_JP)'

Any opinions?

This patch chokes replication when the publisher finds an inconvertible character in a tuple to be sent. For the case, dropping-then-recreating subscription is necessary to go forward.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hello.

This version makes subscriber automatically set its database
encoding to clinet_encoding (if not explicitly set). And changed
the behavior when pg_server_to_client returns NULL to ERROR from
sending original string.

At Wed, 1 Feb 2017 08:39:41 +0000, "Shinoda, Noriyoshi" <noriyoshi.shinoda@hpe.com> wrote in <AT5PR84MB0084A18D3BF1D93B862E95E4EE4D0@AT5PR84MB0084.NAMPRD84.PROD.OUTLOOK.COM>

Thank you for creating patches.
I strongly hope that your patch will be merged into the new
version. Since all databases are not yet based on UTF - 8, I
think conversion of character codes is still necessary.

Thanks.

-----Original Message-----
From: Kyotaro HORIGUCHI [mailto:horiguchi.kyotaro@lab.ntt.co.jp]
Sent: Wednesday, February 01, 2017 3:31 PM
To: Shinoda, Noriyoshi <noriyoshi.shinoda@hpe.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] Logical Replication and Character encoding

At Wed, 01 Feb 2017 12:13:04 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20170201.121304.267734380.horiguchi.kyotaro@lab.ntt.co.jp>

I tried a committed Logical Replication environment. I found
that replication between databases of different encodings did not
convert encodings in character type columns. Is this behavior
correct?

The output plugin for subscription is pgoutput and it currently
doesn't consider encoding but would easiliy be added if desired
encoding is informed.

The easiest (but somewhat seems fragile) way I can guess is,

- Subscriber connects with client_encoding specification and the
output plugin pgoutput decide whether it accepts the encoding
or not. If the subscriber doesn't, pgoutput send data without
conversion.

The attached small patch does this and works with the following
CREATE SUBSCRIPTION.

Oops. It forgets to care conversion failure. It is amended in the
attached patch.

CREATE SUBSCRIPTION sub1 CONNECTION 'host=/tmp port=5432
dbname=postgres client_encoding=EUC_JP' PUBLICATION pub1;

Also we may have explicit negotiation on, for example,
CREATE_REPLICATION_SLOT.

'CREATE_REPLICATION_SLOT sub1 LOGICAL pgoutput ENCODING EUC_JP'

Or output plugin may take options.

'CREATE_REPLICATION_SLOT sub1 LOGICAL pgoutput OPTIONS(encoding EUC_JP)'

Any opinions?

This patch chokes replication when the publisher finds an inconvertible character in a tuple to be sent. For the case, dropping-then-recreating subscription is necessary to go forward.

I think this behavior is right and inevitable in order to protect
subscribers from broken strings.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On 01-02-2017 00:05, Kyotaro HORIGUCHI wrote:

- Subscriber connects with client_encoding specification and the
output plugin pgoutput decide whether it accepts the encoding
or not. If the subscriber doesn't, pgoutput send data without
conversion.

I don't think storage without conversion is an acceptable approach. We
should provide options to users such as ignore tuple or NULL for
column(s) with conversion problem. I wouldn't consider storage data
without conversion because it silently show incorrect data and we
historically aren't flexible with conversion routines.

--
Euler Taveira Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2 February 2017 at 11:45, Euler Taveira <euler@timbira.com.br> wrote:

I don't think storage without conversion is an acceptable approach. We
should provide options to users such as ignore tuple or NULL for
column(s) with conversion problem. I wouldn't consider storage data
without conversion because it silently show incorrect data and we
historically aren't flexible with conversion routines.

pglogical and BDR both require identical encoding; they test for this
during startup and refuse to replicate if the encoding differs.

For the first pass at core I suggest a variant on that policy: require
source and destination encoding to be the same. This should probably
be the first change, since it definitively prevents the issue from
arising.

If time permits we could also allow destination encoding to be UTF-8
(including codepage 65001) with any source encoding. This requires
encoding conversion to be performed, of course.

The downside is that this will impact users who use a common subset of
two encodings. This is most common for Windows-1252 <-> ISO-8859-15
(or -1 if you're old-school) but also arises anywhere the common 7 bit
subset is used. Until we can define an encoding exception policy
though, I think we should defer supporting those and make them a
"later" problem.

--
Craig Ringer 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

Hello,

At Fri, 3 Feb 2017 09:16:47 +0800, Craig Ringer <craig@2ndquadrant.com> wrote in <CAMsr+YGqn2PjJBCY+RjWWTJ4BZ=FhG0REXq1E1nd8_kjaDGoEQ@mail.gmail.com>

On 2 February 2017 at 11:45, Euler Taveira <euler@timbira.com.br> wrote:

I don't think storage without conversion is an acceptable approach. We
should provide options to users such as ignore tuple or NULL for
column(s) with conversion problem. I wouldn't consider storage data
without conversion because it silently show incorrect data and we
historically aren't flexible with conversion routines.

It is possible technically. But changing the behavior of a
subscript and/or publication requires change of SQL syntax. It
seems a bit too late for proposing such a new feature..

IMHO unintentional silent data loss must not be happen so the
default behavior on conversion failure cannot be other than stop
of replication.

pglogical and BDR both require identical encoding; they test for this
during startup and refuse to replicate if the encoding differs.

For the first pass at core I suggest a variant on that policy: require
source and destination encoding to be the same. This should probably
be the first change, since it definitively prevents the issue from
arising.

If the check is performed by BDR, pglogical itself seems to be
allowed to convert strings when the both end have different
encodings in a non-BDR environment. Is this right?

If time permits we could also allow destination encoding to be UTF-8
(including codepage 65001) with any source encoding. This requires
encoding conversion to be performed, of course.

Does this mean that BDR might work on heterogeneous encoding
environemnt? But anyway some encodings (like SJIS) have
caharacters with the same destination in its mapping so BDR
doesn't seem to work with such conversions. So each encoding
might should have a property to inform its usability under BDR
environment, but...

On the other hand, no prolem is seen in encoding conversions in
non-BDR environments. (except the behavior on failure)

The downside is that this will impact users who use a common subset of
two encodings. This is most common for Windows-1252 <-> ISO-8859-15
(or -1 if you're old-school) but also arises anywhere the common 7 bit
subset is used. Until we can define an encoding exception policy
though, I think we should defer supporting those and make them a
"later" problem.

If the conversion is rejected for now, we should check the
encoding identity instead.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 3 Feb. 2017 15:47, "Kyotaro HORIGUCHI" <horiguchi.kyotaro@lab.ntt.co.jp>
wrote:

Hello,

At Fri, 3 Feb 2017 09:16:47 +0800, Craig Ringer <craig@2ndquadrant.com>
wrote in <CAMsr+YGqn2PjJBCY+RjWWTJ4BZ=FhG0REXq1E1nd8_kjaDGoEQ@mail.gmail.com

On 2 February 2017 at 11:45, Euler Taveira <euler@timbira.com.br> wrote:

I don't think storage without conversion is an acceptable approach. We
should provide options to users such as ignore tuple or NULL for
column(s) with conversion problem. I wouldn't consider storage data
without conversion because it silently show incorrect data and we
historically aren't flexible with conversion routines.

It is possible technically. But changing the behavior of a
subscript and/or publication requires change of SQL syntax. It
seems a bit too late for proposing such a new feature..

IMHO unintentional silent data loss must not be happen so the
default behavior on conversion failure cannot be other than stop
of replication.

Agree. Which is why we should default to disallowing mismatched upstream
and downstream encodings. At least to start with.

pglogical and BDR both require identical encoding; they test for this
during startup and refuse to replicate if the encoding differs.

For the first pass at core I suggest a variant on that policy: require
source and destination encoding to be the same. This should probably
be the first change, since it definitively prevents the issue from
arising.

If the check is performed by BDR, pglogical itself seems to be
allowed to convert strings when the both end have different
encodings in a non-BDR environment. Is this right?

Hm. Maybe it's changed since I last looked. We started off disallowing
mismatched encodings anyway.

Note that I'm referring to pglogical, the tool, not "in core logical
replication for postgres"

If time permits we could also allow destination encoding to be UTF-8
(including codepage 65001) with any source encoding. This requires
encoding conversion to be performed, of course.

Does this mean that BDR might work on heterogeneous encoding
environemnt?

No. Here the "we" was meant to be PG core for V10 or later.

But anyway some encodings (like SJIS) have
caharacters with the same destination in its mapping so BDR
doesn't seem to work with such conversions. So each encoding
might should have a property to inform its usability under BDR
environment, but.

PG doesn't allow SJIS as a db encoding. So it doesn't matter here.

The downside is that this will impact users who use a common subset of
two encodings. This is most common for Windows-1252 <-> ISO-8859-15
(or -1 if you're old-school) but also arises anywhere the common 7 bit
subset is used. Until we can define an encoding exception policy
though, I think we should defer supporting those and make them a
"later" problem.

If the conversion is rejected for now, we should check the
encoding identity instead.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Hello,

At Fri, 3 Feb 2017 13:47:54 +0800, Craig Ringer <craig@2ndquadrant.com> wrote in <CAMsr+YFqNLAvdjmgWOMWM9X=FfzcFOL4pLxBeaqtjySbe_UeTg@mail.gmail.com>

On 3 Feb. 2017 15:47, "Kyotaro HORIGUCHI" <horiguchi.kyotaro@lab.ntt.co.jp>
wrote:

Hello,

At Fri, 3 Feb 2017 09:16:47 +0800, Craig Ringer <craig@2ndquadrant.com>
wrote in <CAMsr+YGqn2PjJBCY+RjWWTJ4BZ=FhG0REXq1E1nd8_kjaDGoEQ@mail.gmail.com

On 2 February 2017 at 11:45, Euler Taveira <euler@timbira.com.br> wrote:

I don't think storage without conversion is an acceptable approach. We
should provide options to users such as ignore tuple or NULL for
column(s) with conversion problem. I wouldn't consider storage data
without conversion because it silently show incorrect data and we
historically aren't flexible with conversion routines.

It is possible technically. But changing the behavior of a
subscript and/or publication requires change of SQL syntax. It
seems a bit too late for proposing such a new feature..

IMHO unintentional silent data loss must not be happen so the
default behavior on conversion failure cannot be other than stop
of replication.

Agree. Which is why we should default to disallowing mismatched upstream
and downstream encodings. At least to start with.

pglogical and BDR both require identical encoding; they test for this
during startup and refuse to replicate if the encoding differs.

For the first pass at core I suggest a variant on that policy: require
source and destination encoding to be the same. This should probably
be the first change, since it definitively prevents the issue from
arising.

If the check is performed by BDR, pglogical itself seems to be
allowed to convert strings when the both end have different
encodings in a non-BDR environment. Is this right?

Hm. Maybe it's changed since I last looked. We started off disallowing
mismatched encodings anyway.

Note that I'm referring to pglogical, the tool, not "in core logical
replication for postgres"

Ouch! I'm so sorry for my bad mistake. What I thought that I
mentioned is not pglogical, but pgoutput, the default output
plugin. But what the patch modifies is logical/proto.c. My
correct question was the following.

| Does pglogical requires that the core to reject connections
| with a server with unidentical encoging? Or check condings by
| itself and disconnect by itself?

If time permits we could also allow destination encoding to be UTF-8
(including codepage 65001) with any source encoding. This requires
encoding conversion to be performed, of course.

Does this mean that BDR might work on heterogeneous encoding
environemnt?

No. Here the "we" was meant to be PG core for V10 or later.

But anyway some encodings (like SJIS) have
caharacters with the same destination in its mapping so BDR
doesn't seem to work with such conversions. So each encoding
might should have a property to inform its usability under BDR
environment, but.

PG doesn't allow SJIS as a db encoding. So it doesn't matter here.

Oops. I suppose EUC_JP also has such characters but I'm not sure
now.

The downside is that this will impact users who use a common subset of
two encodings. This is most common for Windows-1252 <-> ISO-8859-15
(or -1 if you're old-school) but also arises anywhere the common 7 bit
subset is used. Until we can define an encoding exception policy
though, I think we should defer supporting those and make them a
"later" problem.

If the conversion is rejected for now, we should check the
encoding identity instead.

Ok, I'll go on this direction for the next patch.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 01/02/17 04:05, Kyotaro HORIGUCHI wrote:

Hello,

At Tue, 31 Jan 2017 12:46:18 +0000, "Shinoda, Noriyoshi" <noriyoshi.shinoda@hpe.com> wrote in <AT5PR84MB0084FAE5976D89CDE9733093EE4A0@AT5PR84MB0084.NAMPRD84.PROD.OUTLOOK.COM>

I tried a committed Logical Replication environment. I found
that replication between databases of different encodings did
not convert encodings in character type columns. Is this
behavior correct?

The output plugin for subscription is pgoutput and it currently
doesn't consider encoding but would easiliy be added if desired
encoding is informed.

The easiest (but somewhat seems fragile) way I can guess is,

- Subscriber connects with client_encoding specification and the
output plugin pgoutput decide whether it accepts the encoding
or not. If the subscriber doesn't, pgoutput send data without
conversion.

Hmm I wonder if we should just make the subscriber send the
client_encoding always (based on server encoding of the subscriber).
That should solve the issue in combination with your patch no?

--
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

At Sat, 4 Feb 2017 21:27:32 +0100, Petr Jelinek <petr.jelinek@2ndquadrant.com> wrote in <bcc7f7e9-f558-b19e-b544-000ba7cf286c@2ndquadrant.com>

Hmm I wonder if we should just make the subscriber send the
client_encoding always (based on server encoding of the subscriber).
That should solve the issue in combination with your patch no?

Yeah, right. I considered that a subscriber might want to set its
own value for that but that is useless.

The attached patch does the following things to just prevent
making a logical replication connection between databases with
inconsistent encodings.

- added client_encoding with subscriber(or standby)'s encoding at
the last of options in libpqrcv_connect.

- CheckLogicalDecodingRequirements refuses connection for a
request with inconsistent encodings.

ERROR: logical replication requires consistent encodings on both side (publisher = UTF8, subscriber = EUC_JP)

We could check this earlier if involving physical replication but
I think this is a matter of logical replication.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On 14/02/17 03:23, Kyotaro HORIGUCHI wrote:

At Sat, 4 Feb 2017 21:27:32 +0100, Petr Jelinek <petr.jelinek@2ndquadrant.com> wrote in <bcc7f7e9-f558-b19e-b544-000ba7cf286c@2ndquadrant.com>

Hmm I wonder if we should just make the subscriber send the
client_encoding always (based on server encoding of the subscriber).
That should solve the issue in combination with your patch no?

Yeah, right. I considered that a subscriber might want to set its
own value for that but that is useless.

The attached patch does the following things to just prevent
making a logical replication connection between databases with
inconsistent encodings.

- added client_encoding with subscriber(or standby)'s encoding at
the last of options in libpqrcv_connect.

- CheckLogicalDecodingRequirements refuses connection for a
request with inconsistent encodings.

ERROR: logical replication requires consistent encodings on both side (publisher = UTF8, subscriber = EUC_JP)

I am not quite convinced that this should be handled by logical decoding
itself. It's quite possible to have output plugins that will handle this
correctly for their use-cases (by doing similar conversion you did in
the original patch) so they should not be prevented doing so.
So it's probably better to check this in the plugin.

I do like the idea of just using client_encoding in libpqrcv_connect though.

--
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

On 2/15/17 17:55, Petr Jelinek wrote:

I am not quite convinced that this should be handled by logical decoding
itself. It's quite possible to have output plugins that will handle this
correctly for their use-cases (by doing similar conversion you did in
the original patch) so they should not be prevented doing so.
So it's probably better to check this in the plugin.

I do like the idea of just using client_encoding in libpqrcv_connect though.

Well, it is sort of a libpq connection, and a proper libpq client should
set the client encoding, and a proper libpq server should do encoding
conversion accordingly. If we just play along with this, it all works
correctly.

Other output plugins are free to ignore the encoding settings (just like
libpq can send binary data in some cases).

The attached patch puts it all together.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2/17/17 10:14, Peter Eisentraut wrote:

Well, it is sort of a libpq connection, and a proper libpq client should
set the client encoding, and a proper libpq server should do encoding
conversion accordingly. If we just play along with this, it all works
correctly.

Other output plugins are free to ignore the encoding settings (just like
libpq can send binary data in some cases).

The attached patch puts it all together.

committed

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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

From: Peter Eisentraut [mailto:peter.eisentraut@2ndquadrant.com]
Sent: Friday, February 24, 2017 1:32 AM
To: Petr Jelinek <petr.jelinek@2ndquadrant.com>; Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: craig@2ndquadrant.com; Shinoda, Noriyoshi <noriyoshi.shinoda@hpe.com>; pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] Logical Replication and Character encoding

On 2/17/17 10:14, Peter Eisentraut wrote:

Well, it is sort of a libpq connection, and a proper libpq client
should set the client encoding, and a proper libpq server should do
encoding conversion accordingly. If we just play along with this, it
all works correctly.

Other output plugins are free to ignore the encoding settings (just
like libpq can send binary data in some cases).

The attached patch puts it all together.

committed

Hi,

Thank you very much for making a new patch. I tried a new committed version.
In the case of PUBLICATION(EUC_JP) and SUBSCRIPTION(UTF-8) environment, it worked as expected. Great!.
However, in the case of PUBLICATION(UTF-8) and SUBSCRIOTION(EUC_JP) environment, the following error was output and the process went down.

- PUBLICATION (UTF-8)
postgres=> INSERT INTO encode1 VALUES (1, 'ascii') ;
INSERT 0 1
postgres=> INSERT INTO encode1 VALUES (2, '漢') ; -- Expect UTF-8 Character 0xE6BCA2 will be convert EUC_JP 0xB4C1
INSERT 0 1

- SUBSCRIPTION (EUC_JP)
postgres=> SELECT * FROM encode1;
c1 | c2
----+-------
1 | ascii
(1 row)

$ tail data.euc/pg_log/postgresql.log
LOG: starting logical replication worker for subscription "sub1"
LOG: logical replication apply for subscription "sub1" has started
ERROR: insufficient data left in message
LOG: worker process: logical replication worker for subscription 16439 (PID 22583) exited with exit code 1

Snapshot:
https://ftp.postgresql.org/pub/snapshot/dev/postgresql-snapshot.tar.gz 2017-02-24 00:28:58
Operating System:
Red Hat Enterprise Linux 7 Update 2 (x86-64)

Regards.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Sorry for the abesnse.

At Fri, 24 Feb 2017 02:43:14 +0000, "Shinoda, Noriyoshi" <noriyoshi.shinoda@hpe.com> wrote in <AT5PR84MB00847ABEA48EAE9A97D51157EE520@AT5PR84MB0084.NAMPRD84.PROD.OUTLOOK.COM>

From: Peter Eisentraut [mailto:peter.eisentraut@2ndquadrant.com]
Sent: Friday, February 24, 2017 1:32 AM
To: Petr Jelinek <petr.jelinek@2ndquadrant.com>; Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: craig@2ndquadrant.com; Shinoda, Noriyoshi <noriyoshi.shinoda@hpe.com>; pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] Logical Replication and Character encoding

On 2/17/17 10:14, Peter Eisentraut wrote:

Well, it is sort of a libpq connection, and a proper libpq client
should set the client encoding, and a proper libpq server should do
encoding conversion accordingly. If we just play along with this, it
all works correctly.

Other output plugins are free to ignore the encoding settings (just
like libpq can send binary data in some cases).

The attached patch puts it all together.

committed

..

However, in the case of PUBLICATION(UTF-8) and SUBSCRIOTION(EUC_JP) environment, the following error was output and the process went down.

...

LOG: starting logical replication worker for subscription "sub1"
LOG: logical replication apply for subscription "sub1" has started
ERROR: insufficient data left in message
LOG: worker process: logical replication worker for subscription 16439 (PID 22583) exited with exit code 1

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.

As the result pg_sendstring is not usable for this case since we
don't have the true length of the string to be sent. So my first
patch did the same thing using pg_server_to_client() explicitly.

That being said, I think that a more important thing is that the
consensus about the policy of logical replication between
databases with different encodings is refusing connection. The
reason for that is it surely breaks BDR usage for some
combinations of encodings.

Anyway the attached patch fixes the current bug about encoding in
logical replication.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

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.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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.

--
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