> On Jun 12, 2026, at 02:48, Masahiko Sawada <[email protected]> wrote:
>
> On Tue, May 26, 2026 at 2:36 AM Aleksander Alekseev
> <[email protected]> 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/
