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

Hi Akshay,

Re-tested v17 on current master (73dfe79fd6) -- all five issues are fixed,
and make check passes here.

I then ran a round-trip check against pg_dump: for each table, replay the
pg_get_table_ddl() output into a clone of the database and diff
"pg_dump -t" of both sides (both sides being pg_dump output, any surviving
diff is semantic, not formatting). Corpus: the create_sql scenarios from
002_pg_dump.pl, then the whole regression database. 483 of 560 tables
round-trip identically; setting aside the documented no-output kinds
(trigger/policy), the rest reduce to:

1) serial columns produce non-replayable DDL -- the output references the
sequence but never creates it:

CREATE TABLE ser (id serial, v text);

SELECT d FROM pg_get_table_ddl('ser'::regclass, owner => false) d;
-- CREATE TABLE public.ser (id integer DEFAULT
nextval('public.ser_id_seq'::regclass) NOT NULL, v text);

CREATE TABLE public.ser (id integer DEFAULT
nextval('public.ser_id_seq'::regclass) NOT NULL, v text);
-- ERROR: relation "public.ser_id_seq" does not exist (on an
empty database)

pg_dump emits CREATE SEQUENCE + OWNED BY + SET DEFAULT. This accounts for
14 regression failures, including whole partition trees.

2) Partition/inheritance children that diverged from their parent don't
survive the PARTITION OF / INHERITS rebuild. Three variants:

- a dropped default silently comes back:

CREATE TABLE dp (a int DEFAULT 99) PARTITION BY LIST (a);
CREATE TABLE dp1 PARTITION OF dp FOR VALUES IN (1);
ALTER TABLE ONLY dp1 ALTER COLUMN a DROP DEFAULT;

SELECT d FROM pg_get_table_ddl('dp'::regclass, owner => false) d;
-- CREATE TABLE public.dp (a integer DEFAULT 99) PARTITION BY LIST (a);
-- CREATE TABLE public.dp1 PARTITION OF public.dp FOR VALUES IN (1);

SELECT pg_get_expr(adbin, adrelid) FROM pg_attrdef
WHERE adrelid = 'dp1'::regclass;
-- source: (0 rows)
-- replayed: 99 => INSERTs into dp1 now get 99, not NULL

- a child's own NOT NULL constraint name is lost:

CREATE TABLE np (a int NOT NULL) PARTITION BY LIST (a);
CREATE TABLE np1 (a int CONSTRAINT np1_nn NOT NULL);
ALTER TABLE np ATTACH PARTITION np1 FOR VALUES IN (1);

SELECT d FROM pg_get_table_ddl('np'::regclass, owner => false) d;
-- CREATE TABLE public.np (a integer NOT NULL) PARTITION BY LIST (a);
-- CREATE TABLE public.np1 PARTITION OF public.np FOR VALUES IN (1);

SELECT conname FROM pg_constraint
WHERE conrelid = 'np1'::regclass AND contype = 'n';
-- source: np1_nn
-- replayed: np_a_not_null

and when the column itself is inherited, the emitted constraint
errors instead of merging:

CREATE TABLE p5 (a int);
CREATE TABLE c5 () INHERITS (p5);
ALTER TABLE c5 ADD CONSTRAINT c5_nn NOT NULL a;
ALTER TABLE p5 ADD CONSTRAINT p5_nn NOT NULL a;

SELECT d FROM pg_get_table_ddl('c5'::regclass, owner => false) d;
-- CREATE TABLE public.c5 () INHERITS (public.p5);
-- ALTER TABLE public.c5 ADD CONSTRAINT c5_nn NOT NULL a;

ALTER TABLE public.c5 ADD CONSTRAINT c5_nn NOT NULL a; --
replay, p5 recreated first
-- ERROR: cannot create not-null constraint "c5_nn" on
column "a" of table "c5"
-- DETAIL: A not-null constraint named "p5_nn" already
exists for this column.

- a partition attached from a table with different column order is
rebuilt in the parent's order:

CREATE TABLE p (a int, b int, c int) PARTITION BY LIST (a);
CREATE TABLE c1 (c int, b int, a int);
ALTER TABLE p ATTACH PARTITION c1 FOR VALUES IN (1);

SELECT d FROM pg_get_table_ddl('p'::regclass, owner => false) d;
-- CREATE TABLE public.p (a integer, b integer, c integer)
PARTITION BY LIST (a);
-- CREATE TABLE public.c1 PARTITION OF public.p FOR VALUES IN (1);
-- => replayed c1 columns are a, b, c (source: c, b, a), so
-- SELECT * / COPY / positional INSERT all shift

pg_dump handles all three: it emits the named constraint inline in the
child's CREATE body (the merge with the inherited constraint at CREATE
time keeps the local name, and attislocal / conislocal / coninhcount all
survive), and falls back to standalone CREATE + ATTACH PARTITION for
shapes a PARTITION OF / INHERITS clause can't express. The first two
variants have lightweight fixes that keep the PARTITION OF / INHERITS
shape --

CREATE TABLE c5 (CONSTRAINT c5_nn NOT NULL a) INHERITS (p5);
CREATE TABLE np1 PARTITION OF np (CONSTRAINT np1_nn NOT NULL a)
FOR VALUES IN (1);

plus a counter-statement for the divergent-default case (ALTER TABLE
ONLY dp1 ALTER COLUMN a DROP DEFAULT). Only the reordered-column case
really needs the standalone shape. The commit message lists child-local
DEFAULT overrides and named NOT NULL constraints as supported, so I'm
treating these as bugs rather than scope cuts.

3) emit_typed_column_storage() (ddlutils.c:2331, 2342) emits ALTER TABLE
without ONLY; SET STORAGE / SET COMPRESSION recurse:

CREATE TYPE mytype AS (a int, b text);
CREATE TABLE typed_t OF mytype;
ALTER TABLE typed_t ALTER COLUMN b SET STORAGE external;
CREATE TABLE tchild () INHERITS (typed_t);
ALTER TABLE ONLY tchild ALTER COLUMN b SET STORAGE main;

ALTER TABLE public.typed_t ALTER COLUMN b SET STORAGE EXTERNAL;
-- replay v17's output
-- => tchild's attstorage flips m -> e

pg_dump uses ALTER TABLE ONLY here. (The other emitted ALTER TABLEs are
fine: OWNER / REPLICA IDENTITY / RLS / SET (options) don't recurse, and
ADD CONSTRAINT must stay ONLY-less since the dump relies on its recursion
to rebuild the children's suppressed inherited copies.)

4) Some per-table state pg_dump preserves is missing. Index statistics
targets:

CREATE TABLE ist (c1 int);
CREATE INDEX ist_idx ON ist ((c1 + 1));
ALTER INDEX ist_idx ALTER COLUMN 1 SET STATISTICS 400;

SELECT d FROM pg_get_table_ddl('ist'::regclass, owner => false) d;
-- CREATE TABLE public.ist (c1 integer);
-- CREATE INDEX ist_idx ON public.ist USING btree (((c1 + 1)));
-- no SET STATISTICS

Same story for ALTER TABLE ... CLUSTER ON (the index comes back without
the indisclustered marker) and for ALTER TABLE ... DISABLE RULE (the rule
is emitted but comes back enabled, which changes behavior -- and rule is
a covered kind).

5) emit_indexes doesn't check indisvalid, so an invalid index is emitted
as a normal one:

CREATE TABLE inv (x int);
INSERT INTO inv VALUES (1), (1);
CREATE UNIQUE INDEX CONCURRENTLY inv_uidx ON inv (x);
-- ERROR: could not create unique index "inv_uidx" (leaves
indisvalid = false)

SELECT d FROM pg_get_table_ddl('inv'::regclass, owner => false) d;
-- CREATE TABLE public.inv (x integer);
-- CREATE UNIQUE INDEX inv_uidx ON public.inv USING btree (x);

pg_dump skips those (getIndexes: "i.indisvalid OR t2.relkind = 'p'").

6) Minor: COMMENT ON and GRANT/REVOKE are not emitted. If that's
intentional -- like trigger/policy -- worth saying so in the doc.

Thanks,
Rui