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

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Rui Zhao <zhaorui126(at)gmail(dot)com>
Cc: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Date: 2026-07-08 11:56:30
Message-ID: CANxoLDfwVcVgFJ7C6W4DUYGt2_r2w81=Q-L8jtiTHE=PiFEN0A@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thank you for the thorough round-trip analysis; this caught real bugs. I
have fixed several of the issues you highlighted, though a few fall outside
this function's current scope.

*Out of Scope (Documented in the Function Description)*
Issue 1 (Serial Columns): Owned sequences are independent catalog objects.
This is the same reason *pg_get_table_ddl* does not emit CREATE TYPE for
composite types used in columns. While the nextval() default is correctly
emitted, the sequence itself requires separate capture. I have added an
explicit note to the documentation to call this out.

Issue 2c (Reordered Partition Columns): You correctly noted that this
scenario requires a standalone CREATE + ATTACH PARTITION fallback.
Detecting column-order divergence and falling back to the non-PARTITION OF
syntax is a larger architecture change, so I have deferred it to a
follow-up task. I have documented this as a known limitation for now.

Issue 6 (COMMENT/GRANT): These are intentionally omitted, consistent with
how we handle triggers and policies. I have added them to the documented
list of exclusions.

The v18 patch is now ready for your review.

On Tue, Jul 7, 2026 at 10:23 PM Rui Zhao <zhaorui126(at)gmail(dot)com> wrote:

> 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
>

Attachment Content-Type Size
v18-0001-Add-pg_get_table_ddl-to-reconstruct-CREATE-TABLE.patch application/octet-stream 249.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2026-07-08 12:04:51 Re: use of SPI by postgresImportForeignStatistics
Previous Message Tatsuro Yamada 2026-07-08 11:54:13 RE: Add enable_groupagg GUC parameter to control GroupAggregate usage