By the way, the v3 failed applying on Head(d35e293878) git am v3-0001-Catalog-not-null-constraints.patch Applying: Catalog not-null constraints error: patch failed: doc/src/sgml/ref/create_table.sgml:77 error: doc/src/sgml/ref/create_table.sgml: patch does not apply error: patch failed: src/backend/commands/tablecmds.c:4834 error: src/backend/commands/tablecmds.c: patch does not apply error: patch failed: src/backend/parser/gram.y:4141 error: src/backend/parser/gram.y: patch does not apply error: patch failed: src/backend/parser/parse_utilcmd.c:2385 error: src/backend/parser/parse_utilcmd.c: patch does not apply Patch failed at 0001 Catalog not-null constraints
Alvaro Herrera <alvhe...@alvh.no-ip.org> 于2024年9月17日周二 01:47写道： > Sadly, there were some other time-wasting events that I failed to > consider, but here's now v3 which has fixed (AFAICS) all the problems > you reported. > > On 2024-Sep-11, jian he wrote: > > > after applying your changes. > > > > in ATExecAddConstraint, ATAddCheckNNConstraint. > > ATAddCheckNNConstraint(wqueue, tab, rel, > > newConstraint, recurse, false, is_readd, > > lockmode); > > if passed to ATAddCheckNNConstraint rel is a partitioned table. > > ATAddCheckNNConstraint itself can recurse to create not-null > pg_constraint > > for itself and it's partitions (children table). > > This is fine as long as we only call ATExecAddConstraint once. > > > > but ATExecAddConstraint itself will recurse, it will call > > the partitioned table and each of the partitions. > > Yeah, this is because ATPrepAddPrimaryKey was queueing SetNotNull nodes > for each column on each children, which is repetitive and causes the > problem you see. That was a leftover from the previous way we handled > PKs; we no longer need it to work that way. I have changed it so that > it queues one constraint addition per column, on the same table that > receives the PK. It now works correctly as far as I can tell. > > Sadly, there's one more pg_dump issue, which causes the pg_upgrade tests > to fail. The problem is that if you have this sequence (taken from > constraints.sql): > > CREATE TABLE notnull_tbl4 (a INTEGER PRIMARY KEY INITIALLY DEFERRED); > CREATE TABLE notnull_tbl4_cld2 (PRIMARY KEY (a) DEFERRABLE) INHERITS > (notnull_tbl4); > > this is dumped by pg_dump in this other way: > > CREATE TABLE public.notnull_tbl4 (a integer NOT NULL); > CREATE TABLE public.notnull_tbl4_cld2 () INHERITS (public.notnull_tbl4); > ALTER TABLE ONLY public.notnull_tbl4_cld2 ADD CONSTRAINT > notnull_tbl4_cld2_pkey PRIMARY KEY (a) DEFERRABLE; > ALTER TABLE ONLY public.notnull_tbl4 ADD CONSTRAINT notnull_tbl4_pkey > PRIMARY KEY (a) DEFERRABLE INITIALLY DEFERRED; > > This is almost exactly the same, except that the PK for > notnull_tbl4_cld2 is created in a separate command ... and IIUC this > causes the not-null constraint to obtain a different name, or a > different inheritance characteristic, and then from the > restored-by-pg_upgrade database, it's dumped by pg_dump separately. > This is what causes the pg_upgrade test to fail. > > Anyway, this made me realize that there is a more general problem, to > wit, that pg_dump is not dumping not-null constraint names correctly -- > sometimes they just not dumped, which is Not Good. I'll have to look > into that once more. > > > (Also: there are still a few additional test stanzas in regress/ that > ought to be removed; also, I haven't re-tested sepgsql, so it's probably > broken ATM.) > > -- > Álvaro Herrera PostgreSQL Developer — > https://www.EnterpriseDB.com/ > -- Thanks, Tender Wang
