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: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Date: 2026-08-31 11:49:33
Message-ID: CANxoLDcedxs9ysmTiOj1ZayfuaWcN8PC0owvSPV2pw1y1uVu_g@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Rui,

Thanks again for the detailed review. I've included both of your patches.

The *v25* patch is ready for review/test.

On Tue, Aug 11, 2026 at 5:21 PM Rui Zhao <zhaorui126(at)gmail(dot)com> wrote:

> Hi Akshay,
>
> v24 applies to master (1d1d7b0e9c), builds warning-free, installcheck
> 246/246.
> All four fixes check out here.
>
> Three things.
>
> 1. The new doc paragraph promises something that doesn't happen. It says
> the
> child constraint names come back if you emit the whole tree:
>
> Round-trip fidelity for these names requires emitting the whole tree
> via the parent table.
>
> Emitting the whole tree doesn't preserve them either -- the child's
> constraint
> isn't in the output at all:
>
> CREATE TABLE pk_p (a int, b int) PARTITION BY LIST (a);
> CREATE TABLE pk_c (a int, b int, CONSTRAINT pk_c_mypk PRIMARY KEY
> (a,b));
> ALTER TABLE pk_p ADD CONSTRAINT pk_p_mypk PRIMARY KEY (a,b);
> ALTER TABLE pk_p ATTACH PARTITION pk_c FOR VALUES IN (1);
>
> SELECT d FROM pg_get_table_ddl('pk_p', owner => false) d;
> -- CREATE TABLE public.pk_p (a integer NOT NULL, b integer NOT NULL)
> -- PARTITION BY LIST (a);
> -- CREATE TABLE public.pk_c PARTITION OF public.pk_p
> -- (CONSTRAINT pk_c_a_not_null NOT NULL a,
> -- CONSTRAINT pk_c_b_not_null NOT NULL b) FOR VALUES IN (1);
> -- ALTER TABLE public.pk_p ADD CONSTRAINT pk_p_mypk PRIMARY KEY (a,
> b);
>
> -- replayed into a fresh database:
> -- source pg_constraint on pk_c: pk_c_mypk
> -- replay pg_constraint on pk_c: pk_c_pkey
>
> Same for UNIQUE (uq_c_myuq) and EXCLUDE (ep1_myexcl), which are not
> mentioned in
> the parent's output either. That is the half you said you could see
> working but
> haven't written yet, so until it is in, the sentence should say the names
> are
> not preserved in either direction rather than point at a workaround that
> isn't
> there.
>
> 2. schema_qualified => false modifies the source when the table is
> partitioned
> with a child in another schema. Attached 0001 fixes this. The docs give
> that
> parameter this purpose:
>
> the script can be replayed under a different search_path to recreate the
> table in another schema
>
> but the child is hard-qualified, and so is the parent reference in its
> PARTITION OF:
>
> CREATE SCHEMA sa; CREATE SCHEMA sb;
> CREATE TABLE sa.yp (a int, b int) PARTITION BY LIST (a);
> CREATE TABLE sb.yc (a int, b int);
> CREATE INDEX yc_myidx ON sb.yc (b);
> CREATE INDEX yp_myidx ON sa.yp (b);
> ALTER TABLE sa.yp ATTACH PARTITION sb.yc FOR VALUES IN (1);
>
> SELECT d FROM pg_get_table_ddl('sa.yp', owner => false,
> schema_qualified => false) d;
> -- CREATE TABLE yp (a integer, b integer) PARTITION BY LIST (a);
> -- CREATE TABLE sb.yc PARTITION OF sa.yp FOR VALUES IN (1);
> -- CREATE INDEX yc_myidx ON sb.yc USING btree (b);
> -- CREATE INDEX yp_myidx ON ONLY yp USING btree (b);
> -- ALTER INDEX yp_myidx ATTACH PARTITION sb.yc_myidx;
>
> sa.yp is written both ways in the one script. Replaying it with
> search_path = sc, against a database that has sa.yp but not yet sb.yc:
>
> SET
> CREATE TABLE
> CREATE TABLE
> CREATE INDEX
> CREATE INDEX
> ERROR: cannot attach index "yc_myidx" as a partition of index
> "yp_myidx"
> DETAIL: Index "yc_myidx" is not an index on any partition of table
> "yp".
>
> SELECT inhparent::regclass, inhrelid::regclass FROM pg_inherits;
> -- sa.yp | sb.yc
> -- sa.yp_myidx | sb.yc_b_idx
>
> sc.yp is left with no partitions, and sa.yp has gained one it did not have
> before -- the second CREATE TABLE attached the child to the source parent.
>
> The child carrying its own schema is right, and the test comment says
> why. But
> it is done by switching the child's whole context to schema_qualified, and
> that
> flag also governs the child's reference back to the parent -- which is in
> the
> base namespace, the one search_path stands in for, so it does not have to
> be
> qualified. 0001 passes the base namespace down instead, which keeps the
> part
> you fixed and drops the part that came along with it, and gives:
>
> -- CREATE TABLE yp (a integer, b integer) PARTITION BY LIST (a);
> -- CREATE TABLE sb.yc PARTITION OF yp FOR VALUES IN (1);
> -- CREATE INDEX yc_myidx ON sb.yc USING btree (b);
> -- CREATE INDEX yp_myidx ON ONLY yp USING btree (b);
> -- ALTER INDEX yp_myidx ATTACH PARTITION sb.yc_myidx;
>
> The run above then completes, with sb.yc attached to sc.yp and sa.yp
> untouched,
> and replaying under search_path = sa still reproduces the original. In
> installcheck it moves one line, in the test that covers this case:
>
> - CREATE TABLE pgtbl_ddl_part_other.pt_c PARTITION OF
> pgtbl_ddl_part_s.pt ...
> + CREATE TABLE pgtbl_ddl_part_other.pt_c PARTITION OF pt ...
>
> 3. The ATTACH PARTITION fix has no test for the shape it fixes. Attached
> 0002
> adds one. The six expected lines the fix moved are all same-schema, where
> the
> old code worked by search_path; a fix that qualified with the base
> namespace
> instead of the child's own would pass them too. One CREATE INDEX on the
> cross-schema partition block already in pg_get_table_ddl.sql covers it:
> with
> 0002 in, putting get_rel_name() back in emit_indexes() turns
>
> ALTER INDEX pt_val_idx ATTACH PARTITION
> pgtbl_ddl_part_other.pt_c_val_idx;
>
> into the bare form, and it is the only one of the seven ATTACH lines that
> is
> cross-schema.
>
> Both patches are on top of v24; installcheck is 246/246 with them.
>
> Regards,
> Rui
>

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Sami Imseih 2026-08-31 12:20:21 Re: WAIT FOR command should do some query jumbling
Previous Message Bertrand Drouvot 2026-08-31 11:38:40 Re: Offline data checksum changes can cause incorrect checksum state on standbys