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

Thanks for the detailed review. I have fixed the issues 1, 2, 4 and 5.

*Issue no 3*: For the tree case (pg_get_table_ddl on the parent): letting
conislocal=false through for PK/UNIQUE/EXCLUSION on a direct partition
child, emitting the child constraint first with ONLY on both sides, then
reusing the ALTER INDEX ... ATTACH PARTITION block I can see that working.

For the standalone-child case, I agree that it's fragile. `CREATE TABLE ...
PARTITION OF auto-creates the child constraint, so an explicit ADD
CONSTRAINT collides. Without a flag on the context to distinguish "called
on child alone" from "called as part of a tree walk", there's no clean way
to rename after the fact. I'll leave that as a documented limitation (added
to func-info.sgml in this version alongside security labels) until the tree
case is in and we can revisit.

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

On Wed, Jul 29, 2026 at 9:37 PM Rui Zhao <zhaorui126(at)gmail(dot)com> wrote:

> Hi Akshay,
>
> Thanks -- v23 builds clean here and installcheck is green (246/246). All
> three
> from my last mail check out: CLUSTER ON no longer dangles on the case I
> posted,
> the self-referential FK errors with the same shape as the REPLICA IDENTITY
> guard, and check_stack_depth() is in the right place in
> emit_partition_children.
>
> Five things after going over the new code.
>
> 1) The FK guard misses index-backed keys.
> fk_backing_constraint_is_blocked()
> scans pg_constraint for a PK/UNIQUE on confrelid, but a foreign key only
> needs a
> unique *index* over the referenced columns -- there needn't be a
> constraint at
> all:
>
> CREATE TABLE u (id int, parent_id int);
> CREATE UNIQUE INDEX u_id_uidx ON u (id);
> ALTER TABLE u ADD CONSTRAINT u_fk FOREIGN KEY (parent_id) REFERENCES u
> (id);
>
> SELECT d FROM pg_get_table_ddl('u', owner => false,
> except_kinds => ARRAY['index']) d;
> -- CREATE TABLE public.u (id integer, parent_id integer);
> -- ALTER TABLE public.u ADD CONSTRAINT u_fk FOREIGN KEY (parent_id)
> -- REFERENCES public.u(id);
> -- replay: ERROR: there is no unique constraint matching given keys
> for
> -- referenced table "u"
>
> The scan finds no candidate, concludes "not blocked", and we are back to
> the
> dangling case. Worth noting REPLICA IDENTITY does catch this same shape --
> a
> table whose replica-identity index is a plain unique index gives
>
> ERROR: REPLICA IDENTITY for table "ri" requires kind "index" to be
> emitted
>
> pg_constraint.conindid on the FK row points straight at the backing index
> in
> both shapes -- u_id_uidx above, and the PK's own index for the
> constraint-backed
> case from my last mail -- so feeding that to the index_ddl_kind() you just
> added
> for CLUSTER ON covers both, and the matching scan can go:
>
> - if (con->confrelid == ctx->relid)
> + if (con->confrelid == ctx->relid &&
> + OidIsValid(con->conindid))
> {
> - TableDdlKind ref_kind;
> + TableDdlKind ref_kind = index_ddl_kind(con->conindid);
>
> - if (fk_backing_constraint_is_blocked(conTup, conRel,
> - ctx, &ref_kind))
> + if (!is_kind_included(ctx, ref_kind))
> {
> @@
> - errdetail("The key references columns constrained by a \"%s\"
> constraint, which is not in the active filter.",
> + errdetail("The key references an index produced by the \"%s\" kind,
> which is not in the active filter.",
>
> with fk_backing_constraint_is_blocked() and its forward declaration
> deleted --
> net -122 lines. The errdetail moves because "constrained by an \"index\"
> constraint" doesn't read right for the plain-index case; that wording now
> matches the REPLICA IDENTITY guard, and the same edit applies to three
> lines in
> pg_get_table_ddl.out. On top of v23 that gives:
>
> self-FK on a plain unique index, except index -> errors (silent
> before)
> self-FK on a PK / UNIQUE, that kind filtered -> still errors
> cross-table FK, backing kind filtered -> still no error
> nothing filtered -> unchanged
> installcheck -> 246/246
>
> 2) CLUSTER ON is dropped even when its backing kind is emitted.
> emit_cluster_on() still opens with
>
> if (!is_kind_included(ctx, TABLE_DDL_KIND_INDEX))
> return;
>
> which predates the new per-index check below it, and for a
> constraint-backed
> clustered index the two now disagree:
>
> 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,
> only_kinds => ARRAY['table','primary_key']) d;
> -- CREATE TABLE public.b (x integer NOT NULL);
> -- ALTER TABLE public.b ADD CONSTRAINT b_pk PRIMARY KEY (x);
>
> The PK is emitted, so the index exists on replay and CLUSTER ON would have
> applied cleanly -- I appended it by hand to check -- but the early return
> drops
> it because "index" isn't in the filter. except_kinds => ARRAY['index']
> behaves
> the same. With index_ddl_kind() in place that outer test is redundant
> anyway,
> since it returns TABLE_DDL_KIND_INDEX for a plain index, so dropping it is
> enough:
>
> List *indexoids;
> ListCell *lc;
>
> - if (!is_kind_included(ctx, TABLE_DDL_KIND_INDEX))
> - return;
> -
> indexoids = RelationGetIndexList(ctx->rel);
>
> With that, a PK- or UNIQUE-backed CLUSTER ON survives only_kinds =>
> ARRAY['table','primary_key'] and except_kinds => ARRAY['index'], while the
> plain-index case and the filtered-backing-kind cases behave as they do now;
> installcheck stays at 246/246, including your new CLUSTER ON tests.
>
> 3) On the deferred naming case -- two notes, then it's your call on scope.
>
> It isn't only EXCLUDE. A custom-named PK or UNIQUE on a partition child
> goes the
> same way, since they all sit behind the same conislocal filter:
>
> 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);
>
> round-trips as pk_c_mypk -> pk_c_pkey, and the UNIQUE equivalent as
> uq_c_myuq -> uq_c_a_b_key.
>
> On the cost: I think it splits in two, and ChooseConstraintName is only
> needed
> for one half.
>
> When the whole tree is emitted -- pg_get_table_ddl on the parent -- the
> child's
> name is still in pg_constraint (ATTACH only clears conislocal and bumps
> coninhcount), so nothing has to be synthesised. It is the shape
> emit_indexes
> already produces for a plain partitioned index:
>
> CREATE INDEX ic_myidx ON public.ic USING btree (b);
> CREATE INDEX ip_myidx ON ONLY public.ip USING btree (b);
> ALTER INDEX public.ip_myidx ATTACH PARTITION ic_myidx;
>
> child first, parent ON ONLY, then ATTACH (that child name is unqualified
> per
> (4), but these are all in one schema). The same works for constraints, and
> the ordering already holds because emit_partition_children runs before
> emit_local_constraints. I hacked it up here to check: let the conislocal
> filter
> through for a partition child's PK/UNIQUE/EXCLUSION, put ONLY on both
> sides, and
> reuse your ALTER INDEX ... ATTACH PARTITION block. ep1_excl, pk_c_mypk and
> uq_c_myuq then come back with conislocal = false and coninhcount = 1,
> matching
> the source, with installcheck still green.
>
> The other half is where your concern lands, and I was wrong to wave it off.
> Called on the child alone, the parent must already exist with its
> constraint, so
> CREATE TABLE ... PARTITION OF creates the child's automatically and an
> explicit
> ADD CONSTRAINT collides:
>
> ERROR: multiple primary keys for table "parted_pk_a" are not allowed
>
> which is what your own regression test gave me before I gated it. Renaming
> after
> the fact does need the generated name, so that path is the fragile one, and
> telling the two apart needs a flag on the context.
>
> If the tree case is worth having on its own, it's the one round-trip
> fidelity
> depends on; the standalone-child call could stay a documented limitation.
>
> 4) Unrelated, but it turned up while comparing against pg_dump:
> emit_indexes() qualifies the parent in ALTER INDEX ... ATTACH PARTITION
> but not
> the child, so a partition child in another schema doesn't replay:
>
> 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) d;
> -- CREATE TABLE sa.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 sa.yp USING btree (b);
> -- ALTER INDEX sa.yp_myidx ATTACH PARTITION yc_myidx;
> -- replay: ERROR: relation "yc_myidx" does not exist
>
> schema_qualified doesn't help, since the child name is never qualified
> either
> way:
>
> SELECT d FROM pg_get_table_ddl('sa.yp', owner => false,
> schema_qualified => true) d;
> -- ALTER INDEX sa.yp_myidx ATTACH PARTITION yc_myidx;
>
> Same-schema cases only work because search_path happens to find the child.
> pg_dump writes both sides qualified -- ALTER INDEX sa.yp_myidx ATTACH
> PARTITION
> sb.yc_myidx; -- and running the child name through
> lookup_relname_for_emit(),
> as you already do for the parent, is enough:
>
> foreach(clc, childIdxOids)
> {
> Oid childIdxOid = lfirst_oid(clc);
> - char *childIdxName = get_rel_name(childIdxOid);
> + char *childIdxName = lookup_relname_for_emit(childIdxOid,
> + ctx->schema_qualified,
> + ctx->base_namespace);
> @@
> parentIdxName,
> - quote_identifier(childIdxName));
> + childIdxName);
>
> With that the cross-schema case replays, schema_qualified => false still
> leaves
> a same-schema child unqualified, and in installcheck it only moves six
> existing
> ATTACH lines to the qualified form.
>
> 5) Two limitations aren't in the documented exclusion list yet. The list
> in func-info.sgml has owned sequences, partition children whose column
> order
> differs from the parent, triggers/policies, and COMMENT/GRANT, but not:
>
> - security labels, which you said back on v21 should be noted as a known
> limitation -- there's no mention of them in the docs as of v23;
> - the partition-child constraint naming above. Its sibling, the column
> reordering, is documented, so a reader would expect to find this one
> there
> too once it's deferred.
>
> Regards,
> Rui
>