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: <7f042818-cc4f-8d4a-2923-d352f5ed10d6@gmail.com>
Date: Tue, 5 Nov 2019 08:37:17 +0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:68.0)
 Gecko/20100101 Thunderbird/68.2.1
MIME-Version: 1.0
In-Reply-To: <20191028.133939.559712139043653003.horikyota.ntt@gmail.com>
Content-Type: multipart/mixed;
 boundary="------------E976EFA228E7BB7B10B28A1A"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------E976EFA228E7BB7B10B28A1A
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

On 2019/10/28 12:39, Kyotaro Horiguchi wrote:
> Hello.
> 
> # The patch no longer applies on the current master. Needs a rebasing.
> 
New patch, rebased on master branch.

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


--------------E976EFA228E7BB7B10B28A1A
Content-Type: text/plain; charset=UTF-8; x-mac-type="0"; x-mac-creator="0";
 name="replidfieldtype.patch"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
 filename="replidfieldtype.patch"

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL2NvbW1hbmRzL3RhYmxlY21kcy5jIGIvc3JjL2Jh
Y2tlbmQvY29tbWFuZHMvdGFibGVjbWRzLmMKaW5kZXggMDU1OTNmMzMxNi4uMDJmOGRiZWFi
ZiAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvY29tbWFuZHMvdGFibGVjbWRzLmMKKysrIGIv
c3JjL2JhY2tlbmQvY29tbWFuZHMvdGFibGVjbWRzLmMKQEAgLTE3NCw2ICsxNzQsOCBAQCB0
eXBlZGVmIHN0cnVjdCBBbHRlcmVkVGFibGVJbmZvCiAJTGlzdAkgICAqY2hhbmdlZENvbnN0
cmFpbnREZWZzOwkvKiBzdHJpbmcgZGVmaW5pdGlvbnMgb2Ygc2FtZSAqLwogCUxpc3QJICAg
KmNoYW5nZWRJbmRleE9pZHM7CS8qIE9JRHMgb2YgaW5kZXhlcyB0byByZWJ1aWxkICovCiAJ
TGlzdAkgICAqY2hhbmdlZEluZGV4RGVmczsJLyogc3RyaW5nIGRlZmluaXRpb25zIG9mIHNh
bWUgKi8KKwlPaWQJCSAgIGNoYW5nZWRSZXBsSWRlbnRPaWQ7CS8qIE9JRCBvZiBpbmRleCB0
byByZXNldCBSRVBMSUNBIElERU5USUZZICovCisJY2hhcgkgICAqY2hhbmdlZFJlcGxJZGVu
dERlZjsJLyogc3RyaW5nIGRlZmluaXRpb25zIG9mIHNhbWUgKi8KIH0gQWx0ZXJlZFRhYmxl
SW5mbzsKIAogLyogU3RydWN0IGRlc2NyaWJpbmcgb25lIG5ldyBjb25zdHJhaW50IHRvIGNo
ZWNrIGluIFBoYXNlIDMgc2NhbiAqLwpAQCAtNDU5LDYgKzQ2MSw3IEBAIHN0YXRpYyBPYmpl
Y3RBZGRyZXNzIEFURXhlY0FsdGVyQ29sdW1uVHlwZShBbHRlcmVkVGFibGVJbmZvICp0YWIs
IFJlbGF0aW9uIHJlbCwKIAkJCQkJCQkJCQkgICBBbHRlclRhYmxlQ21kICpjbWQsIExPQ0tN
T0RFIGxvY2ttb2RlKTsKIHN0YXRpYyB2b2lkIFJlbWVtYmVyQ29uc3RyYWludEZvclJlYnVp
bGRpbmcoT2lkIGNvbm9pZCwgQWx0ZXJlZFRhYmxlSW5mbyAqdGFiKTsKIHN0YXRpYyB2b2lk
IFJlbWVtYmVySW5kZXhGb3JSZWJ1aWxkaW5nKE9pZCBpbmRvaWQsIEFsdGVyZWRUYWJsZUlu
Zm8gKnRhYik7CitzdGF0aWMgdm9pZCBSZW1lbWJlclJlcGxpY2FJZGVudEZvclJlYnVpbGRp
bmcoT2lkIGluZG9pZCwgQWx0ZXJlZFRhYmxlSW5mbyAqdGFiKTsKIHN0YXRpYyB2b2lkIEFU
UG9zdEFsdGVyVHlwZUNsZWFudXAoTGlzdCAqKndxdWV1ZSwgQWx0ZXJlZFRhYmxlSW5mbyAq
dGFiLAogCQkJCQkJCQkgICBMT0NLTU9ERSBsb2NrbW9kZSk7CiBzdGF0aWMgdm9pZCBBVFBv
c3RBbHRlclR5cGVQYXJzZShPaWQgb2xkSWQsIE9pZCBvbGRSZWxJZCwgT2lkIHJlZlJlbElk
LApAQCAtMTA3MTUsNiArMTA3MTgsOSBAQCBBVEV4ZWNBbHRlckNvbHVtblR5cGUoQWx0ZXJl
ZFRhYmxlSW5mbyAqdGFiLCBSZWxhdGlvbiByZWwsCiAJCQkJCXsKIAkJCQkJCUFzc2VydChm
b3VuZE9iamVjdC5vYmplY3RTdWJJZCA9PSAwKTsKIAkJCQkJCVJlbWVtYmVySW5kZXhGb3JS
ZWJ1aWxkaW5nKGZvdW5kT2JqZWN0Lm9iamVjdElkLCB0YWIpOworCisJCQkJCQlpZiAoUmVs
YXRpb25HZXRGb3JtKHJlbCktPnJlbHJlcGxpZGVudD09UkVQTElDQV9JREVOVElUWV9JTkRF
WCkKKwkJCQkJCQlSZW1lbWJlclJlcGxpY2FJZGVudEZvclJlYnVpbGRpbmcoZm91bmRPYmpl
Y3Qub2JqZWN0SWQsIHRhYik7CiAJCQkJCX0KIAkJCQkJZWxzZSBpZiAocmVsS2luZCA9PSBS
RUxLSU5EX1NFUVVFTkNFKQogCQkJCQl7CkBAIC0xMDc0OSw5ICsxMDc1NSwxOCBAQCBBVEV4
ZWNBbHRlckNvbHVtblR5cGUoQWx0ZXJlZFRhYmxlSW5mbyAqdGFiLCBSZWxhdGlvbiByZWws
CiAJCQkJfQogCiAJCQljYXNlIE9DTEFTU19DT05TVFJBSU5UOgotCQkJCUFzc2VydChmb3Vu
ZE9iamVjdC5vYmplY3RTdWJJZCA9PSAwKTsKLQkJCQlSZW1lbWJlckNvbnN0cmFpbnRGb3JS
ZWJ1aWxkaW5nKGZvdW5kT2JqZWN0Lm9iamVjdElkLCB0YWIpOwotCQkJCWJyZWFrOworCQkJ
CXsKKwkJCQkJT2lkIGluZElkOworCisJCQkJCUFzc2VydChmb3VuZE9iamVjdC5vYmplY3RT
dWJJZCA9PSAwKTsKKwkJCQkJUmVtZW1iZXJDb25zdHJhaW50Rm9yUmVidWlsZGluZyhmb3Vu
ZE9iamVjdC5vYmplY3RJZCwgdGFiKTsKKworCQkJCQlpbmRJZCA9IGdldF9jb25zdHJhaW50
X2luZGV4KGZvdW5kT2JqZWN0Lm9iamVjdElkKTsKKwkJCQkJaWYgKE9pZElzVmFsaWQoaW5k
SWQpKQorCQkJCQkJUmVtZW1iZXJSZXBsaWNhSWRlbnRGb3JSZWJ1aWxkaW5nKGluZElkLCB0
YWIpOworCisJCQkJCWJyZWFrOworCQkJCX0KIAogCQkJY2FzZSBPQ0xBU1NfUkVXUklURToK
IAkJCQkvKiBYWFggc29tZWRheSBzZWUgaWYgd2UgY2FuIGNvcGUgd2l0aCByZXZpc2luZyB2
aWV3cyAqLwpAQCAtMTEwNzUsNiArMTEwOTAsMzYgQEAgUmVtZW1iZXJDb25zdHJhaW50Rm9y
UmVidWlsZGluZyhPaWQgY29ub2lkLCBBbHRlcmVkVGFibGVJbmZvICp0YWIpCiAJfQogfQog
CisvKgorICogU3Vicm91dGluZSBmb3IgQVRFeGVjQWx0ZXJDb2x1bW5UeXBlOiByZW1lbWJl
ciB0aGF0IGEgcmVwbGljYSBpZGVudGlmeQorICogbmVlZHMgdG8gYmUgcmVzZXQgKHdoaWNo
IHdlIG1pZ2h0IGFscmVhZHkga25vdykuCisgKi8KK3N0YXRpYyB2b2lkCitSZW1lbWJlclJl
cGxpY2FJZGVudEZvclJlYnVpbGRpbmcoT2lkIGluZG9pZCwgQWx0ZXJlZFRhYmxlSW5mbyAq
dGFiKQoreworCWNoYXIJICAgKmRlZnN0cmluZzsKKworCS8qCisJICogVGhpcyBkZS1kdXBs
aWNhdGlvbiBjaGVjayBpcyBjcml0aWNhbCBmb3IgdHdvIGluZGVwZW5kZW50IHJlYXNvbnM6
IHdlCisJICogbXVzdG4ndCB0cnkgdG8gcmVjcmVhdGUgdGhlIHNhbWUgY29uc3RyYWludCB0
d2ljZSwgYW5kIGlmIGEgY29uc3RyYWludAorCSAqIGRlcGVuZHMgb24gbW9yZSB0aGFuIG9u
ZSBjb2x1bW4gd2hvc2UgdHlwZSBpcyB0byBiZSBhbHRlcmVkLCB3ZSBtdXN0CisJICogY2Fw
dHVyZSBpdHMgZGVmaW5pdGlvbiBzdHJpbmcgYmVmb3JlIGFwcGx5aW5nIGFueSBvZiB0aGUg
Y29sdW1uIHR5cGUKKwkgKiBjaGFuZ2VzLiAgcnVsZXV0aWxzLmMgd2lsbCBnZXQgY29uZnVz
ZWQgaWYgd2UgYXNrIGFnYWluIGxhdGVyLgorCSAqLworCWlmIChPaWRJc1ZhbGlkKHRhYi0+
Y2hhbmdlZFJlcGxJZGVudE9pZCkpCisJCXJldHVybjsKKworCS8qIE9LLCBjYXB0dXJlIHRo
ZSBjb25zdHJhaW50J3MgZXhpc3RpbmcgZGVmaW5pdGlvbiBzdHJpbmcgKi8KKwlkZWZzdHJp
bmcgPSBwZ19nZXRfcmVwbGlkZW50ZGVmX2NvbW1hbmQoaW5kb2lkKTsKKworCS8qIG5vdCBh
IHJlcGxpY2EgaWRlbnRpZnkgKi8KKwlpZiAoZGVmc3RyaW5nPT1OVUxMKQorCQlyZXR1cm47
CisKKwl0YWItPmNoYW5nZWRSZXBsSWRlbnRPaWQgPSBpbmRvaWQ7CisJdGFiLT5jaGFuZ2Vk
UmVwbElkZW50RGVmID0gZGVmc3RyaW5nOworfQorCiAvKgogICogU3Vicm91dGluZSBmb3Ig
QVRFeGVjQWx0ZXJDb2x1bW5UeXBlOiByZW1lbWJlciB0aGF0IGFuIGluZGV4IG5lZWRzCiAg
KiB0byBiZSByZWJ1aWx0ICh3aGljaCB3ZSBtaWdodCBhbHJlYWR5IGtub3cpLgpAQCAtMTEy
MjIsNiArMTEyNjcsMTUgQEAgQVRQb3N0QWx0ZXJUeXBlQ2xlYW51cChMaXN0ICoqd3F1ZXVl
LCBBbHRlcmVkVGFibGVJbmZvICp0YWIsIExPQ0tNT0RFIGxvY2ttb2RlKQogCQlhZGRfZXhh
Y3Rfb2JqZWN0X2FkZHJlc3MoJm9iaiwgb2JqZWN0cyk7CiAJfQogCisJaWYgKE9pZElzVmFs
aWQodGFiLT5jaGFuZ2VkUmVwbElkZW50T2lkKSkKKwl7CisJCU9pZCByZWxpZCA9IEluZGV4
R2V0UmVsYXRpb24odGFiLT5jaGFuZ2VkUmVwbElkZW50T2lkLCBmYWxzZSk7CisJCUFUUG9z
dEFsdGVyVHlwZVBhcnNlKEludmFsaWRPaWQsIHJlbGlkLCBJbnZhbGlkT2lkLAorCQkJCQkJ
CSB0YWItPmNoYW5nZWRSZXBsSWRlbnREZWYsCisJCQkJCQkJIHdxdWV1ZSwgbG9ja21vZGUs
IHRhYi0+cmV3cml0ZSk7CisJfQorCisKIAkvKgogCSAqIEl0IHNob3VsZCBiZSBva2F5IHRv
IHVzZSBEUk9QX1JFU1RSSUNUIGhlcmUsIHNpbmNlIG5vdGhpbmcgZWxzZSBzaG91bGQKIAkg
KiBiZSBkZXBlbmRpbmcgb24gdGhlc2Ugb2JqZWN0cy4KQEAgLTExMzczLDYgKzExNDI3LDEx
IEBAIEFUUG9zdEFsdGVyVHlwZVBhcnNlKE9pZCBvbGRJZCwgT2lkIG9sZFJlbElkLCBPaWQg
cmVmUmVsSWQsIGNoYXIgKmNtZCwKIAkJCQkJCQkJCQkJIE5JTCwKIAkJCQkJCQkJCQkJIGNv
bi0+Y29ubmFtZSk7CiAJCQkJfQorCQkJCWVsc2UgaWYgKGNtZC0+c3VidHlwZSA9PSBBVF9S
ZXBsaWNhSWRlbnRpdHkpCisJCQkJeworCQkJCQl0YWItPnN1YmNtZHNbQVRfUEFTU19NSVND
XSA9CisJCQkJCQlsYXBwZW5kKHRhYi0+c3ViY21kc1tBVF9QQVNTX01JU0NdLCBjbWQpOwor
CQkJCX0KIAkJCQllbHNlIGlmIChjbWQtPnN1YnR5cGUgPT0gQVRfU2V0Tm90TnVsbCkKIAkJ
CQl7CiAJCQkJCS8qCmRpZmYgLS1naXQgYS9zcmMvYmFja2VuZC91dGlscy9hZHQvcnVsZXV0
aWxzLmMgYi9zcmMvYmFja2VuZC91dGlscy9hZHQvcnVsZXV0aWxzLmMKaW5kZXggM2U2NDM5
MGQ4MS4uYzVlZmU5M2MwYyAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvdXRpbHMvYWR0L3J1
bGV1dGlscy5jCisrKyBiL3NyYy9iYWNrZW5kL3V0aWxzL2FkdC9ydWxldXRpbHMuYwpAQCAt
MTg1NCw2ICsxODU0LDQ0IEBAIHBnX2dldF9wYXJ0Y29uc3RyZGVmX3N0cmluZyhPaWQgcGFy
dGl0aW9uSWQsIGNoYXIgKmFsaWFzbmFtZSkKIAlyZXR1cm4gZGVwYXJzZV9leHByZXNzaW9u
KChOb2RlICopIGNvbnN0cl9leHByLCBjb250ZXh0LCB0cnVlLCBmYWxzZSk7CiB9CiAKKy8q
CisgKiBwZ19nZXRfcmVwbGlkZW50ZGVmX2NvbW1hbmQKKyAqCisgKiBSZXR1cm5zIHRoZSBk
ZWZpbml0aW9uIGZvciB0aGUgUkVQTElDQSBJREVOVElUWSwgaWUsIGV2ZXJ5dGhpbmcgdGhh
dCBuZWVkcyB0bworICogYXBwZWFyIGFmdGVyICJBTFRFUiBUQUJMRSAuLi4gUkVQTElDQSBJ
REVOVElUWSBVU0lORyBJTkRFWCA8aW5kZXhuYW1lPiIuCisgKi8KK2NoYXIgKnBnX2dldF9y
ZXBsaWRlbnRkZWZfY29tbWFuZChPaWQgaW5kSWQpCit7CisJT2lkCQkJcmVsSWQ7CisJSGVh
cFR1cGxlCWh0X2lkeDsKKwlGb3JtX3BnX2luZGV4IGlkeHJlYzsKKworCVN0cmluZ0luZm9E
YXRhIGJ1ZjsKKworCWh0X2lkeCA9IFNlYXJjaFN5c0NhY2hlMShJTkRFWFJFTElELCBPYmpl
Y3RJZEdldERhdHVtKGluZElkKSk7CisJaWYgKCFIZWFwVHVwbGVJc1ZhbGlkKGh0X2lkeCkp
CisJCWVsb2coRVJST1IsICJjYWNoZSBsb29rdXAgZmFpbGVkIGZvciBpbmRleCAldSIsIGlu
ZElkKTsKKworCWlkeHJlYyA9IChGb3JtX3BnX2luZGV4KSBHRVRTVFJVQ1QoaHRfaWR4KTsK
KwkvKiBub3QgYSByZXBsaWNhIGlkZW50aWZ5ICovCisJaWYgKCFpZHhyZWMtPmluZGlzcmVw
bGlkZW50KQorCXsKKwkJUmVsZWFzZVN5c0NhY2hlKGh0X2lkeCk7CisJCXJldHVybiBOVUxM
OworCX0KKwlyZWxJZCA9IGlkeHJlYy0+aW5kcmVsaWQ7CisJUmVsZWFzZVN5c0NhY2hlKGh0
X2lkeCk7CisKKwlpbml0U3RyaW5nSW5mbygmYnVmKTsKKwlhcHBlbmRTdHJpbmdJbmZvU3Ry
aW5nKCZidWYsICJBTFRFUiBUQUJMRSAiKTsKKworCWFwcGVuZFN0cmluZ0luZm9TdHJpbmco
JmJ1ZiwgZ2VuZXJhdGVfcXVhbGlmaWVkX3JlbGF0aW9uX25hbWUocmVsSWQpKTsKKwlhcHBl
bmRTdHJpbmdJbmZvU3RyaW5nKCZidWYsICIgUkVQTElDQSBJREVOVElUWSBVU0lORyBJTkRF
WCAiKTsKKwlhcHBlbmRTdHJpbmdJbmZvU3RyaW5nKCZidWYsIHF1b3RlX2lkZW50aWZpZXIo
Z2V0X3JlbF9uYW1lKGluZElkKSkpOworCisJcmV0dXJuIGJ1Zi5kYXRhOworfQorCiAvKgog
ICogcGdfZ2V0X2NvbnN0cmFpbnRkZWYKICAqCmRpZmYgLS1naXQgYS9zcmMvaW5jbHVkZS91
dGlscy9ydWxldXRpbHMuaCBiL3NyYy9pbmNsdWRlL3V0aWxzL3J1bGV1dGlscy5oCmluZGV4
IGQzNGNhZDJmNGIuLjcxYzNkYTEzMTYgMTAwNjQ0Ci0tLSBhL3NyYy9pbmNsdWRlL3V0aWxz
L3J1bGV1dGlscy5oCisrKyBiL3NyYy9pbmNsdWRlL3V0aWxzL3J1bGV1dGlscy5oCkBAIC0y
NCw2ICsyNCw4IEBAIGV4dGVybiBjaGFyICpwZ19nZXRfaW5kZXhkZWZfY29sdW1ucyhPaWQg
aW5kZXhyZWxpZCwgYm9vbCBwcmV0dHkpOwogZXh0ZXJuIGNoYXIgKnBnX2dldF9wYXJ0a2V5
ZGVmX2NvbHVtbnMoT2lkIHJlbGlkLCBib29sIHByZXR0eSk7CiBleHRlcm4gY2hhciAqcGdf
Z2V0X3BhcnRjb25zdHJkZWZfc3RyaW5nKE9pZCBwYXJ0aXRpb25JZCwgY2hhciAqYWxpYXNu
YW1lKTsKIAorZXh0ZXJuIGNoYXIgKnBnX2dldF9yZXBsaWRlbnRkZWZfY29tbWFuZChPaWQg
aW5kSWQpOworCiBleHRlcm4gY2hhciAqcGdfZ2V0X2NvbnN0cmFpbnRkZWZfY29tbWFuZChP
aWQgY29uc3RyYWludElkKTsKIGV4dGVybiBjaGFyICpkZXBhcnNlX2V4cHJlc3Npb24oTm9k
ZSAqZXhwciwgTGlzdCAqZHBjb250ZXh0LAogCQkJCQkJCQlib29sIGZvcmNlcHJlZml4LCBi
b29sIHNob3dpbXBsaWNpdCk7Cg==
--------------E976EFA228E7BB7B10B28A1A--