Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).
Dean Rasheed <dean.a.rasheed@gmail.com>
From: Dean Rasheed <dean.a.rasheed@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Aleksander Alekseev <aleksander@timescale.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>,
Amul Sul <sulamul@gmail.com>, vignesh C <vignesh21@gmail.com>
Date: 2024-03-08T11:18:41Z
Lists: pgsql-hackers
On Thu, 7 Mar 2024 at 17:32, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > > This seems to work okay. > Yes, this looks good. I tested it against CREATE TABLE ... LIKE, and it worked as expected. It might be worth adding a test case for that, to ensure that it doesn't get broken in the future. Do we also want a test case that does what pg_dump would do: - Add a NOT NULL constraint - Add a deferrable PK constraint - Drop the NOT NULL constraint - Check the column is still not nullable Looking at rel.h, I think that the new field should probably come after rd_pkindex, under the comment "data managed by RelationGetIndexList", and have its own comment. Also, if I'm nitpicking, the new field and local variables should use the term "deferrable" rather than "deferred". A DEFERRABLE constraint can be set to be either DEFERRED or IMMEDIATE within a transaction, but "deferrable" is the right term to use to describe the persistent property of an index/constraint that can be deferred. (The same objection applies to the field name "indimmediate", but it's too late to change that.) Also, for neatness/consistency, the new field should probably be reset in load_relcache_init_file(), alongside rd_pkindex, though I don't think it can matter in practice. Regards, Dean
Commits
-
Revert structural changes to not-null constraints
- 6f8bb7c1e961 17.0 cited
-
Admit deferrable PKs into rd_pkindex, but flag them as such
- 270af6f0df76 17.0 landed
-
Add the notion of REPLICA IDENTITY for a table.
- 07cacba983ef 9.4.0 cited