Subject: Re: Restore replication settings when modifying a field type
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: pgsql-hackers@postgresql.org
References: <c70fcab2-4866-0d9f-1d01-e75e189db342@gmail.com>
 <20191028.133939.559712139043653003.horikyota.ntt@gmail.com>
From: Quan Zongliang <quanzongliang@gmail.com>
Message-ID: <e3d023a5-6731-143e-16f7-4342f6a2f200@gmail.com>
Date: Fri, 1 Nov 2019 09:41:57 +0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:60.0)
 Gecko/20100101 Thunderbird/60.9.0
MIME-Version: 1.0
In-Reply-To: <20191028.133939.559712139043653003.horikyota.ntt@gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2019/10/28 12:39, Kyotaro Horiguchi wrote:
> Hello.
> 
> # The patch no longer applies on the current master. Needs a rebasing.
> 
> At Sat, 26 Oct 2019 16:50:48 +0800, Quan Zongliang <quanzongliang@gmail.com> wrote in
>> In fact, the replication property of the table has not been modified,
>> and it is still 'i'(REPLICA_IDENTITY_INDEX). But the previously
>> specified index property 'indisreplident' is set to false because of
>> the rebuild.
> 
> I suppose that the behavior is intended. Change of column types on the
> publisher side can break the agreement on replica identity with
> subscribers. Thus replica identity setting cannot be restored
> unconditionally. For (somewhat artifitial :p) example:
> 
> P=# create table t (c1 integer, c2 text unique not null);
> P=# alter table t replica identity using index t_c2_key;
> P=# create publication p1 for table t;
> P=# insert into t values (0, '00'), (1, '01');
> S=# create table t (c1 integer, c2 text unique not null);
> S=# alter table t replica identity using index t_c2_key;
> S=# create subscription s1 connection '...' publication p1;
> 
> Your patch allows change of the type of c2 into integer.
> 
> P=# alter table t alter column c2 type integer using c2::integer;
> P=# update t set c1 = c1 + 1 where c2 = '01';
> 
> This change doesn't affect perhaps as expected.
> 
> S=# select * from t;
>   c1 | c2
> ----+----
>    0 | 00
>    1 | 01
> (2 rows)
> 
> 
>> So I developed a patch. If the user modifies the field type. The
>> associated index is REPLICA IDENTITY. Rebuild and restore replication
>> settings.
> 
> Explicit setting of replica identity premises that they are sure that
> the setting works correctly. Implicit rebuilding after a type change
> can silently break it.
> 
> At least we need to guarantee that the restored replica identity
> setting is truly compatible with all existing subscribers. I'm not
> sure about potential subscribers..
> 
> Anyway I think it is a problem that replica identity setting is
> dropped silently. Perhaps a message something like "REPLICA IDENTITY
> setting is lost, please redefine after confirmation of compatibility
> with subscribers." is needed.
> 
In fact, the scene we encountered is like this. The field of a user's 
table is of type "smallint", and it turns out that this range is not 
sufficient. So change it to "int". At this point, the REPLICA IDENTITY 
is lost and the user does not realize it. When they found out, the 
logical replication for this period of time did not output normally. 
Users have to find other ways to get the data back.
The logical replication of this user is to issue standard SQL statements 
to other relational databases using the plugin developed by himself. And 
they have thousands of tables to replicate.
So I think this patch is appropriate in this scenario. As for the 
matching problem between publishers and subscribers, I'm afraid it's 
hard to solve here. If this is not a suitable modification, I can 
withdraw it. And see if there's a better way.

If necessary, I'll modify it again. Rebase to the master branch.

> regards.
>