| From: | Yuchen Li <liyuchen_xyz(at)163(dot)com> |
|---|---|
| To: | Antonin Houska <ah(at)cybertec(dot)at>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
| Subject: | Re: repack: fix a bug to reject deferrable primary key fallback for concurrent mode |
| Date: | 2026-04-21 07:03:39 |
| Message-ID: | 258b6521-100e-48da-a7c4-b069ba3df007@163.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 4/21/2026 2:09 PM, Antonin Houska wrote:
> Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>>> On Apr 20, 2026, at 22:52, Antonin Houska <ah(at)cybertec(dot)at> wrote:
>>>
>>> I'm just thinking if it's worth a separate error message.
>>> RelationGetIndexList() just ignores the deferrable PK
>>>
>>> if (replident == REPLICA_IDENTITY_DEFAULT && OidIsValid(pkeyIndex) && !pkdeferrable)
>>> relation->rd_replidindex = pkeyIndex;
>>>
>>> and if there's no other suitable index, the result is that there is no
>>> identity index for the table. So the change attached here should be consistent
>>> with this approach.
>> Thanks for your review. I guess you read the v1 patch. In v2, I have switched to use GetRelationIdentityOrPK() that Zhijie suggested, which has covered RelationGetIndexList() and all checks, so that code is simplified, and there is no longer a separate error message.
> Yes, this looks like the best approach. Sorry for missing v2.
>
The patch looks good to me. Using GetRelationIdentityOrPK() makes the
check match the intended replica identity semantics more closely, and
the added regression coverage looks useful.
Regards,
Yuchen Li
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Antonin Houska | 2026-04-21 07:09:00 | Re: logical: fix recomputation required LSN on restart_lsn-only advancement |
| Previous Message | Chao Li | 2026-04-21 07:01:31 | Re: Cleanup shadows variable warnings, round 1 |