[PATCH] REPLICA IDENTITY USING INDEX accepts column with invalid NOT NULL
Hi,
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,
Ante Kresic
Staff Engineer | Tiger Data
TigerData.com
Attachments:
0001-Reject-REPLICA-IDENTITY-USING-INDEX-on-column-with-i.patchtext/x-patch; charset=US-ASCII; name=0001-Reject-REPLICA-IDENTITY-USING-INDEX-on-column-with-i.patchDownload+68-1
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]https://commitfest.postgresql.org/patch/6801/. Please add yourself as the author.
[1]: https://commitfest.postgresql.org/patch/6801/
--
Best regards,
Aleksander Alekseev