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

Hi Rui,

Thanks again for the review. I've included your patche.

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

On Tue, Sep 8, 2026 at 8:49 PM Rui Zhao <zhaorui126(at)gmail(dot)com> wrote:

> Hi Akshay,
>
> v25 applies to master (798bdcae89), builds warning-free, make check
> 244/244. The three points from my last mail are all in: the doc note now
> says the child constraint names are not preserved in either direction, the
> schema_qualified => false script replays under search_path = sc with sb.yc
> attached to sc.yp and sa.yp untouched, and the cross-schema ATTACH line is
> in the expected output.
>
> Round-trip against pg_dump, re-run on v25 and, alongside it, on v22 so
> the two are like for like: 508 of the 563 top-level tables in the
> regression database come back identical on both, and the same 55 tables
> fall outside for the same reasons. 30 are objects the function doesn't
> emit by design (triggers, grants, policies, comments, an owned sequence),
> 14 are the copy's DROP ... CASCADE taking a serial's sequence or a
> function with it, and the remaining 11 are the partition child name and
> column-order cases already deferred, three name collisions in the copy,
> and one I hadn't looked at before:
>
> 1. An unlogged table's identity sequence that was SET LOGGED comes back
> unlogged:
>
> CREATE UNLOGGED TABLE idu (a int GENERATED ALWAYS AS IDENTITY);
> ALTER SEQUENCE idu_a_seq SET LOGGED;
> SELECT d FROM pg_get_table_ddl('idu', owner => false) d;
> -- CREATE UNLOGGED TABLE public.idu (a integer GENERATED ALWAYS AS
> -- IDENTITY NOT NULL);
>
> -- replayed: idu_a_seq has relpersistence = u; the source has p
>
> emit_identity_sequence_alterations() emits ALTER SEQUENCE ... SET UNLOGGED
> when the sequence is unlogged (ddlutils.c:2990). The sequence starts out
> with the table's persistence, so the thing to test against is the table,
> not LOGGED: on an unlogged table the sequence is unlogged by default and
> the change worth emitting is the opposite one. This is the mirror of
> Zsolt's 11b, and the regression database has it (identity_dump_unlogged).
> Attached 0001 compares against the table and emits SET LOGGED or SET
> UNLOGGED accordingly, with the mirror test next to 11b. With it the
> round-trip is 509 of 563 and make check stays 244/244.
>
> Two notes on the changes that aren't from my patches.
>
> 2. Switching the inline CHECKs to pg_get_constraintdef_body() also drops
> PRETTYFLAG_INDENT, so a CHECK with a CASE no longer brings the deparser's
> newlines into an otherwise single-line CREATE TABLE. Good, and the
> inline path now matches the table-level one; just noting it since the
> change is visible and nothing in the test exercises it.
>
> 3. The ReleaseSysCache() calls added in front of ereport(ERROR) and
> aclcheck_error() aren't needed. The pin is registered with the resource
> owner when the tuple is fetched, and the abort path releases it
> (ResOwnerReleaseCatCache), which is what every other ereport(ERROR) with
> a syscache tuple in hand relies on. There are seven of these in
> ddlutils.c now; harmless, just dead code.
>
> From my side this is the last round. With 0001 in, every table in the
> regression database is accounted for, and v22 through v25 haven't moved
> on that corpus in either direction. I have no open items; I'll re-run
> the round-trip on later versions only to check for regressions.
>
> Regards,
> Rui
>