Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements

Thanks for the review, Rui. Except for issue 1b), the others are fixed.
The *v20* patch is ready for review.

On Fri, Jul 10, 2026 at 10:06 AM Rui Zhao <zhaorui126(at)gmail(dot)com> wrote:

> Hi Akshay,
>
> Thanks for v19. I re-ran the pg_dump round-trip check (the 002_pg_dump.pl
> scenarios plus the whole regression database) on current master
> (2e6578292a).
> The five fixes hold up: SET STORAGE/COMPRESSION now use ONLY, invalid
> indexes
> are skipped, SET STATISTICS / CLUSTER ON / DISABLE RULE are emitted, and
> the
> child-default and named-NOT NULL cases I sent all round-trip now. 494 of
> 562
> tables match pg_dump exactly (was 483/560 on v17). Three things came out of
> the run below; the third is a side effect of that invalid-index skip.
>
> 1) The named-NOT NULL handling for inheritance/partition children still
> isn't
> complete. It works when the child constraint has a user-defined name, but
> the
> corpus turns up two ways it still goes wrong.
>
> 1a) For an auto-named inherited NOT NULL it emits an out-of-line ADD
> CONSTRAINT
> that collides with the one INHERITS/PARTITION OF already propagates, giving
> non-replayable DDL:
>
> CREATE TABLE nt (a int PRIMARY KEY);
> CREATE TABLE ntc (PRIMARY KEY (a) DEFERRABLE) INHERITS (nt);
>
> SELECT d FROM pg_get_table_ddl('ntc'::regclass, owner => false) d;
> -- CREATE TABLE public.ntc () INHERITS (public.nt);
> -- ALTER TABLE public.ntc ADD CONSTRAINT ntc_a_not_null NOT NULL a;
> -- ALTER TABLE public.ntc ADD CONSTRAINT ntc_pkey PRIMARY KEY (a)
> DEFERRABLE;
>
> -- replaying that, with nt already restored first so INHERITS
> propagates
> -- its nt_a_not_null down to ntc:
> ERROR: cannot create not-null constraint "ntc_a_not_null" on
> column "a" of table "ntc"
> DETAIL: A not-null constraint named "nt_a_not_null" already
> exists for this column.
>
> (notnull_tbl4_cld2 in the regression suite fails the same way.)
>
> 1b) Even where it does replay, a partition attached from a standalone table
> keeps its name but comes back with the wrong constraint locality, because
> the
> constraint is folded into PARTITION OF instead of a standalone CREATE +
> ATTACH:
>
> CREATE TABLE mp (a int NOT NULL) PARTITION BY LIST (a);
> CREATE TABLE mp1 (a int CONSTRAINT mp1_nn NOT NULL);
> ALTER TABLE mp ATTACH PARTITION mp1 FOR VALUES IN (1);
> -- source mp1.mp1_nn: conislocal = false (the ATTACH merged it
> into the parent's)
>
> SELECT d FROM pg_get_table_ddl('mp'::regclass, owner => false) d;
> -- CREATE TABLE public.mp (a integer NOT NULL) PARTITION BY LIST (a);
> -- CREATE TABLE public.mp1 PARTITION OF public.mp (CONSTRAINT
> mp1_nn NOT NULL a) FOR VALUES IN (1);
>
> Declaring the constraint inline makes it local, so mp1.mp1_nn comes back
> with
> conislocal = true, and the behavior diverges once the parent's NOT NULL is
> dropped:
>
> ALTER TABLE mp ALTER a DROP NOT NULL;
> -- source: mp1.a is no longer NOT NULL; reconstructed: mp1.a stays NOT
> NULL
>
> pg_dump reconstructs both faithfully. For reference, this is where it puts
> each constraint:
>
> - In the CREATE TABLE body: NOT NULL, and validated CHECK.
> - As a separate ALTER TABLE ... ADD CONSTRAINT: PRIMARY KEY, UNIQUE,
> EXCLUDE,
> FOREIGN KEY, and NOT VALID CHECK.
>
> The split is the same on plain tables, inheritance children and partitions;
> only the table shape differs:
>
> - A plain-inheritance child keeps CREATE ... INHERITS and emits only its
> locally-owned constraints (purely-inherited NOT NULL/CHECK are left to
> INHERITS); the NOT NULL goes in the body unnamed so it auto-names, e.g.
> for the 1a case: CREATE TABLE ntc (NOT NULL a) INHERITS (nt); ...
>
> - A partition is never dumped with CREATE ... PARTITION OF; it is a
> standalone
> CREATE TABLE plus ALTER TABLE ONLY parent ATTACH PARTITION, so the
> child's
> column order, constraint names and conislocal come from its own
> definition
> plus the merge ATTACH performs.
>
> That partition path is the same standalone+ATTACH that the reordered-column
> case you deferred needs, so 1b likely folds into that follow-up. I'll
> leave the exact approach to you.
>
> 2) A NOT VALID CHECK constraint loses its NOT VALID flag on round-trip. The
> constraint is emitted inline in CREATE TABLE, where it is validated
> against the
> (empty) table, so the reconstructed constraint is marked validated:
>
> CREATE TABLE nv (a int);
> ALTER TABLE nv ADD CONSTRAINT chk CHECK (a < 50) NOT VALID;
>
> SELECT d FROM pg_get_table_ddl('nv'::regclass, owner => false) d;
> -- CREATE TABLE public.nv (a integer, CONSTRAINT chk CHECK ((a <
> 50)) NOT VALID);
>
> -- source nv.chk: convalidated = false
> -- reconstructed nv.chk: convalidated = true
>
> pg_dump emits NOT VALID constraints as a separate ALTER TABLE ... ADD
> CONSTRAINT ... NOT VALID for this reason.
>
> 3) The invalid-index skip added for the earlier finding also drops
> legitimate
> partitioned indexes. emit_indexes now has:
>
> /* Skip invalid indexes; they may be left over from a failed
> CREATE INDEX CONCURRENTLY. */
> if (!idxform->indisvalid)
> {
> ReleaseSysCache(indTup);
> continue;
> }
>
> but a partitioned index on the parent is normally indisvalid = false (e.g.
> one
> created with ON ONLY, or before all child indexes are attached), so the
> whole
> index is lost:
>
> CREATE TABLE p (a int) PARTITION BY RANGE (a);
> CREATE TABLE p1 PARTITION OF p FOR VALUES FROM (0) TO (100);
> CREATE INDEX ON ONLY p (a); -- p_a_idx.indisvalid = false
>
> SELECT d FROM pg_get_table_ddl('p'::regclass, owner => false) d;
> -- CREATE TABLE public.p (a integer) PARTITION BY RANGE (a);
> -- CREATE TABLE public.p1 PARTITION OF public.p FOR VALUES FROM
> (0) TO (100);
>
> No CREATE INDEX at all; pg_dump keeps it as "CREATE INDEX p_a_idx ON ONLY
> public.p USING btree (a)". That's the pg_dump condition I quoted earlier --
> "i.indisvalid OR t2.relkind = 'p'"; the relkind = 'p' half needs to stay so
> partitioned indexes aren't filtered out along with the failed-CIC ones.
>
> Thanks,
> Rui
>