Re: [PATCH] REPLICA IDENTITY USING INDEX accepts column with invalid NOT NULL
Re: [PATCH] REPLICA IDENTITY USING INDEX accepts column with invalid NOT NULL
От:
Álvaro Herrera <alvherre@kurilemu.de>
Дата:
On 2026-Jun-11, Masahiko Sawada wrote: > It would be better to factor the into a common function that checks > whether the column has a valid NOT NULL constraint, to remove the code > duplication. I agree, we should have a function like bool notNullConstraintValidated(Oid relid, AttrNumber attnum) in pg_constraint.c, and use it in the various places where we only care about the constraint validation status. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
Re: [PATCH] REPLICA IDENTITY USING INDEX accepts column with invalid NOT NULL
От:
Álvaro Herrera <alvherre@kurilemu.de>
Дата:
On 2026-Jun-12, Álvaro Herrera wrote: > On 2026-Jun-11, Masahiko Sawada wrote: > > > It would be better to factor the into a common function that checks > > whether the column has a valid NOT NULL constraint, to remove the code > > duplication. > > I agree, we should have a function like > bool > notNullConstraintValidated(Oid relid, AttrNumber attnum) > in pg_constraint.c, and use it in the various places where we only care > about the constraint validation status. I take that back. I think this change would warrant merging this new code with the existing verifyNotNullPKCompatible(), which wants to do pretty much the same; but that's probably a larger refactoring that should be done in pg20 only. So maybe we can just do the simple thing (Ante's patch, mostly, I think) for this backpatchable bug fix, then change things more fully later on. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
Re: [PATCH] REPLICA IDENTITY USING INDEX accepts column with invalid NOT NULL
От:
Aleksander Alekseev <aleksander@tigerdata.com>
Дата:
Hi Ante, > ALTER TABLE ... REPLICA IDENTITY USING INDEX checks key columns by > reading attnotnull, but since a379061a22a8 (PG 18, NOT NULL NOT VALID) > attnotnull is set even when the constraint is unvalidated. An index on > a column that actually contains NULLs is therefore accepted as replica > identity: > > CREATE TABLE t (id int); > INSERT INTO t VALUES (1), (NULL), (2); > ALTER TABLE t ADD CONSTRAINT id_nn NOT NULL id NOT VALID; > CREATE UNIQUE INDEX t_idx ON t(id); > ALTER TABLE t REPLICA IDENTITY USING INDEX t_idx; -- accepted > -- relreplident => 'i' > > This would cause data divergence on UPDATE/DELETE targeting NULL-keyed rows. > > Patch follows same pattern as d9ffc27291f (the same bugfix for identity columns); > after the attnotnull check, look up the constraint and reject if !convalidated. Thanks for the patch. This clearly seems to be a bug. The patch is well written and has regression tests. The tests pass, I also tested under Valgrind. All in all the patch looks good to me. I added the patch to the commitfest application to make sure it's not going to be lost [1]. Please add yourself as the author. [1]: https://commitfest.postgresql.org/patch/6801/ -- Best regards, Aleksander Alekseev
Re: [PATCH] REPLICA IDENTITY USING INDEX accepts column with invalid NOT NULL
От:
Chao Li <li.evan.chao@gmail.com>
Дата:
> On Jun 12, 2026, at 02:48, Masahiko Sawada wrote:
>
> On Tue, May 26, 2026 at 2:36 AM Aleksander Alekseev
> wrote:
>>
>> Hi Ante,
>>
>>> ALTER TABLE ... REPLICA IDENTITY USING INDEX checks key columns by
>>> reading attnotnull, but since a379061a22a8 (PG 18, NOT NULL NOT VALID)
>>> attnotnull is set even when the constraint is unvalidated. An index on
>>> a column that actually contains NULLs is therefore accepted as replica
>>> identity:
>>>
>>> CREATE TABLE t (id int);
>>> INSERT INTO t VALUES (1), (NULL), (2);
>>> ALTER TABLE t ADD CONSTRAINT id_nn NOT NULL id NOT VALID;
>>> CREATE UNIQUE INDEX t_idx ON t(id);
>>> ALTER TABLE t REPLICA IDENTITY USING INDEX t_idx; -- accepted
>>> -- relreplident => 'i'
>>>
>>> This would cause data divergence on UPDATE/DELETE targeting NULL-keyed rows.
>>>
>>> Patch follows same pattern as d9ffc27291f (the same bugfix for identity columns);
>>> after the attnotnull check, look up the constraint and reject if !convalidated.
>>
>
> Good catch!
>
>> Thanks for the patch. This clearly seems to be a bug.
>>
>> The patch is well written and has regression tests. The tests pass, I
>> also tested under Valgrind. All in all the patch looks good to me.
>
> Regarding the patch,
>
> + /*
> + * attnotnull is set even for invalid (NOT VALID) not-null
> + * constraints, which do not prove the column is
> null-free, so verify
> + * that the underlying constraint is validated.
> + */
> + {
> + HeapTuple contup;
> + Form_pg_constraint conForm;
> +
> + contup =
> findNotNullConstraintAttnum(RelationGetRelid(rel), attno);
> + if (!HeapTupleIsValid(contup))
> + elog(ERROR, "cache lookup failed for
> not-null constraint on column \"%s\" of relation \"%s\"",
> + NameStr(attr->attname),
> RelationGetRelationName(rel));
> +
> + conForm = (Form_pg_constraint) GETSTRUCT(contup);
> + if (!conForm->convalidated)
> + ereport(ERROR,
> +
> errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("index \"%s\"
> cannot be used as replica identity because column \"%s\" has an
> invalid not-null constraint",
> +
> RelationGetRelationName(indexRel),
> +
> NameStr(attr->attname)),
> + /*- translator: second %s is a
> constraint characteristic such as NOT VALID */
> + errdetail("The
> constraint \"%s\" is marked %s.",
> +
> NameStr(conForm->conname), "NOT VALID"),
> + errhint("You might
> need to validate it using %s.",
> + "ALTER
> TABLE ... VALIDATE CONSTRAINT"));
> + heap_freetuple(contup);
> + }
>
> It would be better to factor the into a common function that checks
> whether the column has a valid NOT NULL constraint, to remove the code
> duplication.
>
> Regards,
>
> --
> Masahiko Sawada
> Amazon Web Services: https://aws.amazon.com
Hi Masahiko-san,
As you are on this topic, I reported a related bug about REPLICA IDENTITY USING INDEX some time ago: when ALTER TABLE ... ALTER COLUMN TYPE rebuilds an index used as replica identity on a partitioned table, the rebuilt partition indexes can silently lose their indisreplident marking. The parent index keeps its replica identity flag, but the corresponding child partition index is rebuilt with the flag cleared, leaving the partition without the expected replica identity.
Would you please take a look at it as well when you get a chance? This is an old bug, not for v19, so no rush at all. It’s here: https://commitfest.postgresql.org/patch/6440/.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
