Hi Akshay,
Sorry for the slow turnaround -- the World Cup got the better of my evenings.
A) Round-trip against pg_dump. I re-ran it on v22 (master 8767a10cb8): for each
of the 563 top-level tables in the regression database I ran pg_get_table_ddl,
dropped and recreated that table from its output in a copy of the database, and
diffed it against pg_dump. 504 of 563 round-trip clean, and the rate has climbed
with each round -- 483/560 at v17, 494/562 at v19, 501/562 at v21,
504/563 now --
with installcheck passing throughout.
For the record, here is what the remaining 59 are:
- 31 are objects pg_get_table_ddl doesn't emit by design (triggers, grants,
policies, comments, owned sequences). The pg_dump comparison flags them, but
that is expected.
- 17 are artifacts of testing (I replay into a copy with the table dropped),
not defects: 14 rely on an owned sequence or dependent function
that goes with
the drop and isn't part of the table's own DDL; 3 pick up a "1" suffix on a
NOT NULL constraint whose name a sibling table still holds. The
emitted DDL is
correct.
- 10 are column reordering on partition or inheritance children -- the
reordered-column limitation you deferred, plus its
inheritance-child variant.
- 1 is real but cosmetic: a partition child's named EXCLUDE constraint isn't
preserved -- it replays fine, only the child's name is regenerated. Just the
parent constraint is emitted:
CREATE TABLE ep (a int4range, b int4range) PARTITION BY LIST (a);
CREATE TABLE ep1 (a int4range, b int4range,
CONSTRAINT ep1_excl EXCLUDE USING gist (a WITH
=, b WITH &&));
ALTER TABLE ep ADD CONSTRAINT ep_excl EXCLUDE USING gist (a WITH
=, b WITH &&);
ALTER TABLE ep ATTACH PARTITION ep1 FOR VALUES IN ('[0,1)');
SELECT d FROM pg_get_table_ddl('ep', owner => false) d;
-- CREATE TABLE public.ep (a int4range, b int4range) PARTITION
BY LIST (a);
-- CREATE TABLE public.ep1 PARTITION OF public.ep FOR VALUES IN
('[0,1)');
-- ALTER TABLE public.ep ADD CONSTRAINT ep_excl EXCLUDE USING
gist (a WITH =, b WITH &&);
ep1's own constraint (ep1_excl) is never emitted, so on replay the parent's
ADD CONSTRAINT recreates it auto-named ep1_a_b_excl instead of ep1_excl.
So the substantive round-trip gaps come down to the reordered columns (your
deferred limitation) and that one EXCLUDE naming case; none breaks replay. (1b,
the conislocal locality case from my v19 pass -- a hand-built one, not in the
corpus above -- is the same family and still open, as you'd noted.)
B) only_kinds/except_kinds filtering. The cross-kind dependency check is only
half there. When a statement leans on an object another kind emits -- a
constraint's backing index -- and you filter that kind out, REPLICA IDENTITY
USING INDEX catches it and errors cleanly ('requires kind "primary_key" to be
emitted'); CLUSTER ON and a self-referential FK don't, and dangle:
CREATE TABLE b (x int);
ALTER TABLE b ADD CONSTRAINT b_pk PRIMARY KEY (x);
ALTER TABLE b CLUSTER ON b_pk;
SELECT d FROM pg_get_table_ddl('b', owner => false,
except_kinds => ARRAY['primary_key']) d;
-- CREATE TABLE public.b (x integer NOT NULL);
-- ALTER TABLE public.b CLUSTER ON b_pk;
-- replay: ERROR: index "b_pk" for table "b" does not exist
CREATE TABLE t (id int PRIMARY KEY, parent_id int REFERENCES t(id));
SELECT d FROM pg_get_table_ddl('t', owner => false,
except_kinds => ARRAY['primary_key']) d;
-- CREATE TABLE public.t (id integer NOT NULL, parent_id integer);
-- ALTER TABLE public.t ADD CONSTRAINT t_parent_id_fkey
-- FOREIGN KEY (parent_id) REFERENCES public.t(id);
-- replay: ERROR: there is no unique constraint matching given keys for
-- referenced table "t"
except_kinds => ARRAY['unique'] does the same. Both want the guard REPLICA
IDENTITY already has -- the regression test even covers it for all three index
kinds; CLUSTER ON and the FK case just aren't wired to it. A plain-index
CLUSTER ON and a cross-table FK are fine as they are -- the first drops with the
index kind, the second gets its key from the referenced table's own output.
C) One small code note. emit_partition_children() recurses into
pg_get_table_ddl_internal() once per partition-tree level:
children = find_inheritance_children(ctx->relid, AccessShareLock);
foreach(lc, children)
{
...
CHECK_FOR_INTERRUPTS();
...
childstmts = pg_get_table_ddl_internal(&childctx);
For user-triggerable recursion like this the convention is to put a
check_stack_depth() next to that CHECK_FOR_INTERRUPTS(). It does fail cleanly
today -- I threw a 12000-level partition tree at it and got the usual "stack
depth limit exceeded" -- but only because some callee happens to check; the
recursion itself never does.
Regards,
Rui
