Hi Matthias, Thanks for digging into this. I agree with the direction of v1: the check should use RelationGetReplicaIndex() so it matches what logical decoding actually requires, rather than GetRelationIdentityOrPK(), which falls back to the primary key while decoding does not. That mismatch is exactly what lets a table with a since-dropped REPLICA IDENTITY USING INDEX slip past the check and then fail during catch-up with "incomplete delete info".
Two problems with the way v1 restructures the deferrable-primary-key case,
though -- it moves that special case out of the "no identity index" branch
and turns it into an unconditional "must have a non-deferrable PK" test:
1) It now rejects a valid, repackable table. With an explicit replica
identity index and no primary key, ident_idx is valid, so we skip the
first error; but RelationGetPrimaryKeyIndex(rel, false) is InvalidOid, so
the second check fires:
CREATE TABLE rt (a int not null, b text);
CREATE UNIQUE INDEX rt_a ON rt(a);
ALTER TABLE rt REPLICA IDENTITY USING INDEX rt_a;
REPACK (CONCURRENTLY) rt;
-- master: ok
-- v1: ERROR: cannot execute REPACK (CONCURRENTLY) on relation "rt"
-- DETAIL: ... does not support deferrable primary keys.
-- HINT: Use ALTER TABLE ... REPLICA IDENTITY USING INDEX ...
The table has no primary key at all (deferrable or otherwise), and the
hint suggests doing exactly what it already did.
2) The genuine deferrable case loses its message. For a table with a
deferrable PK and no explicit identity, RelationGetReplicaIndex() is
InvalidOid, so it hits the first (generic) error, and the deferrable-
specific message in the second check is now unreachable.
So the deferrable special case ends up firing for the wrong table and not
firing for the right one. I think it just wants to stay a reason for "no
identity index", i.e. inside that branch, and the fix can be the one-line
swap that keeps the original structure:
- ident_idx = GetRelationIdentityOrPK(rel);
+ ident_idx = RelationGetReplicaIndex(rel);
if (!OidIsValid(ident_idx))
{
if (OidIsValid(rel->rd_pkindex) && rel->rd_ispkdeferrable)
ereport(... deferrable primary keys ...);
ereport(... no identity index ...);
}
I built that and checked the three cases: explicit identity index without a
PK still repacks, a dropped-identity-index table is now refused up front
with "no identity index" (instead of failing mid-repack), and the deferrable
case keeps its own message. Happy to post it as a patch if useful.
On Chao's suggestion to block DROP INDEX of a replica identity index: it's a
reasonable hardening idea, but I don't think it removes the need to fix the
check here. The guard lives in RemoveRelations(), so it only covers the
DROP INDEX command -- an index backing a constraint is still dropped through
the dependency path, so e.g.
ALTER TABLE t DROP CONSTRAINT t_a_key; -- t_a_key is the RI index
leaves relreplident = 'i' with the index gone, same stale state. (I
confirmed that on a build with that patch.) And pre-existing tables in that
state, from before such a change or via pg_upgrade, would still reach the
old code path. So REPACK needs to handle the state gracefully regardless;
blocking the drop, if wanted, seems like a separate discussion (and a
behavior change, since dropping such an index is allowed today).
For what it's worth I also agree with the note that decoding can't just fall
back to the PK -- the identity is a contract with subscribers, and an 'i'
table deliberately isn't using the PK, so substituting it would log the
wrong columns.
On Fri, Aug 28, 2026 at 3:31 AM Matthias van de Meent
<[email protected]> wrote:
>
> On Thu, 27 Aug 2026 at 20:26, Nathan Bossart <[email protected]> wrote:
> >
> > I don't fully understand the mechanics of this one, but here is a
> > reproducer:
> >
> > CREATE TABLE t (a INT PRIMARY KEY, b INT, c TEXT);
> > INSERT INTO t SELECT g, g, repeat('x', 1000) FROM generate_series(1,
> > 1000000) g;
> > CREATE UNIQUE INDEX i ON t (a);
> > ALTER TABLE t REPLICA IDENTITY USING INDEX i;
> > DROP INDEX i;
> > REPACK (CONCURRENTLY) t;
> >
> > -- in a separate session, while REPACK is still running
> > DELETE FROM t WHERE a = 1;
> >
> > This produces the following ERROR from the REPACK command:
> >
> > ERROR: incomplete delete info
> > CONTEXT: slot "pg_repack_34213", output plugin "pgrepack", in the
> > change callback, associated LSN 0/A6CC1ED0
> > REPACK decoding worker
> >
> > I think this can be addressed by verifying the index exists in
> > check_concurrent_repack_requirements() and erroring out if it doesn't.
>
> I think the issue is caused by the following: REPACK's check has the
> incorrect assumption that REPLICA IDENTITY USING INDEX either reverts
> to DEFAULT or falls back to the behaviour of DEFAULT if the identity
> index gets dropped, and thus uses GetRelationIdentityOrPK(), which
> hides a lack of replica identity index. The issue shows up due to the
> following garden path of data flows:
>
> 1. A table with REPLICA IDENTITY USING INDEX doesn't fall back to
> REPLICA IDENTITY DEFAULT once the identity index is dropped.
> 2. In the catcache, the table won't fall back to rd_replidindex =
> pkeyIndex when replident='i', but instead will set
> rd_replidindex=InvalidOid.
> See the tail end of RelationGetIndexList.
> 3. Then, in heap_delete, it calls ExtractReplicaIdentity() to find the
> key of the deleted tuple.
> 3a. ExtractR_I_() checks the identity key attributes from
> RelationGetIndexAttrBitmap(..., INDEX_ATTR_BITMAP_IDENTITY_KEY), which
> also only uses rd_replidindex, and doesn't fall back to the primary
> key index's attributes.
> 3b. If ExtractR_I_() doesn't have identity key attributes, it returns NULL
> 3c. heap_delete thus doesn't have any logical identity attributes to
> log, and treats the delete operation as any non-logical deletion when
> logging the data.
> 4. Finally, the DELETE record gets decoded, and the logical plugin
> finds out that no logical key data was included, and promptly ERRORs
> out.
>
> The attached patch is a blind shot that I suspect will fix the issue.
>
>
> Kind regards,
>
> Matthias van de Meent
> Databricks (https://www.databricks.com)
--
Regards,
Ewan Young
v2-0001-Repack-Fix-replica-identity-index-check.patch
Description: Binary data
