Re: [PATCH] Improve REPACK (CONCURRENTLY) error messages for unsupported configurations

> On May 29, 2026, at 02:35, Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
>
> On 2026-May-27, Chao Li wrote:
>
>>> 0002 -- check_concurrent_repack_requirements() reports the same
>>> generic "no identity index" error for several distinct
>>> cases, two of which are misleading: REPLICA IDENTITY FULL
>>> (which is set, but the hint says there is no identity), and
>>> a deferrable PK as the only identity (skipped per commit
>>> 832e220d99a, but the hint suggests adding an index that
>>> already exists). Distinguish these cases.
>>
>> When I was working on 832e220d99a, I actually considered for more
>> detailed error messages, but I ended up giving up. I think we should
>> be careful about adding more branches here unless the existing message
>> is causing significant confusion in practice.
>
> I pushed this one too (well, something close to it anyway), because I
> think the replica identity issue could be an (unnecessary) usability
> tripwire.
>
> I'm curious to know why you gave up on this, if you want to share more.
>

Sure. As the committed version is slightly different from the original patch, my comments below are still based on the original patch:

1. I am not against improving the error message in principle, but I'm worried about the maintainability of distinguishing all the possible reasons here.

GetRelationIdentityOrPK() currently returns InvalidOid for several cases, so the patch tries to infer the reason afterwards from relreplident, rd_ispkdeferrable, rd_pkindex, etc. That works for the current cases, but it also means that whenever the REPACK (CONCURRENTLY) requirements change, these extra checks may need to be revisited. For example, if FULL is supported in the future, the FULL specific error would need to be removed or changed.

2. I also think the hint for the deferrable primary key case may be too specific:
```
Use ALTER TABLE ... ALTER CONSTRAINT to make the primary key NOT DEFERRABLE, or use ALTER TABLE ... REPLICA IDENTITY USING INDEX to designate another index.
```

Those are useful suggestions in many cases, but they are not necessarily the only possible solutions. So I was not sure if we should provide such a specific hint.

3. Especially for the FULL check, I have a pending patch [1] that may potentially allow REPLICA_IDENTITY_DEFAULT to fall back to FULL. If something like that eventually happens, this kind of detailed check could become misleading. My general concern is that the more detailed we make these inferred errors, the more fragile they may become when future behavior changes.

BTW, for patch [1], this is not an imagined feature. The requirement came from real customer operations. So, Álvaro, if you could help there, I would greatly appreciate it. That is a long thread, so no rush.

[1] https://postgr.es/m/CAEoWx2mMorbMwjKbT4YCsjDyL3r9Mp+z0bbK57VZ+OkJTgJQVQ@mail.gmail.com

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/