> On May 29, 2026, at 02:35, Álvaro Herrera <[email protected]> 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+okjtgj...@mail.gmail.com
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
