| From: | Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(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-06-22 12:40:53 |
| Message-ID: | CANxoLDe_GOXyyZs7GN3VUJoT+o1DwqZGhid-TWWvS0D8d5ggYw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
You're right, and thanks for spotting this. The existing pattern in
pg_proc.dat for variadic-text functions (e.g., jsonb_delete,
json_extract_path) uses _text at the variadic position in both proargtypes
and proallargtypes, with provariadic => 'text'. That is the convention
documented by the sanity check in src/test/regress/sql/opr_sanity.sql.
The same issue applies to *pg_get_role_ddl*, *pg_get_tablespace_ddl* (both
variants), and *pg_get_database_ddl*, but that will require a separate
patch.
The v9 patch is ready for review.
On Mon, Jun 22, 2026 at 12:25 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
> > On Jun 22, 2026, at 14:26, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
> wrote:
> >
> > Thanks for the review; you're right, `includes_foreign_keys=false` on
> its own is a half-measure. Re-running with the default to add FKs back
> collides with the existing CREATE TABLE, UNIQUE indexes, etc.
> >
> > I've added an only_foreign_keys option (boolean, default false) as the
> natural complement of includes_foreign_keys=false. When set to true, the
> function emits only the ALTER TABLE ... ADD CONSTRAINT ... FOREIGN KEY
> statements and suppresses everything else (CREATE TABLE, owner, indexes,
> non-FK constraints, rules, statistics, replica identity, RLS toggles).
> Partition-child recursion still runs so child FKs are reached too.
> Combining `only_foreign_keys=true` with `includes_foreign_keys=false` is
> rejected upfront since it would produce no output.
> >
> > The documentation paragraph for `includes_foreign_keys` now directs
> users to `only_foreign_keys` as the intended second pass. Regression
> coverage adds three cases: the FK-only emission for your cons example, the
> zero-row result for a table without FKs, and the error path.
> >
> > The v8 patch is ready for review.
> >
> > On Sat, Jun 20, 2026 at 1:15 AM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
> wrote:
> > The previous features all look good to me, I only have one question
> > for the new flag.
> >
> > > Calling
> > > pg_get_table_ddl(t, 'includes_foreign_keys', 'false') now emits
> everything
> > > except FOREIGN KEY constraints. This covers the multi-tenant clone
> > > workflow: create tables first without cross-table references, then
> re-run
> > > with the default to add the constraints once all targets exist.
> >
> > I think this feature needs a bit more documentation, an
> > "only_foreign_keys" flag, or both.
> >
> > CREATE TABLE refd (id int PRIMARY KEY);
> > CREATE TABLE cons (a int CHECK(a>0), b int UNIQUE, c int REFERENCES
> refd(id));
> >
> > -- pass 1: running without foreign keys
> > SELECT * FROM pg_get_table_ddl('cons','includes_foreign_keys','false');
> > -- execute everything
> >
> > -- loading data
> >
> > -- pass 2: running with everything
> > SELECT * FROM pg_get_table_ddl('cons','includes_foreign_keys','true');
> > -- ERROR: relation "cons" already exists (and the unique constraint
> > also collides)
> >
> > I could do a "grep FOREIGN KEY" before executing (unless it's a tricky
> > schema where that phrase appears elsewhere), or since psql continues
> > on error, it will simply work if I accept a significant error noise,
> > but then the documentation should be clear about this limitation.
> > Following the documented approach and getting a bunch of unexpected
> > errors could be confusing for users.
> >
> >
> > <v8-0001-Add-pg_get_table_ddl-to-reconstruct-CREATE-TABLE.patch>
>
> I have a comment, or maybe a question:
> ```
> +{ oid => '8215', descr => 'get DDL to recreate a table',
> + proname => 'pg_get_table_ddl', prorows => '50', provariadic => 'text',
> + proisstrict => 'f', proretset => 't', provolatile => 's', proparallel
> => 'r',
> + pronargdefaults => '1', prorettype => 'text',
> + proargtypes => 'regclass text', proallargtypes => '{regclass,text}',
> + proargmodes => '{i,v}', proargdefaults => '{NULL}',
> + prosrc => 'pg_get_table_ddl' },
> ```
>
> Since provariadic is text, I wonder if proallargtypes should be
> {regclass,_text}, with _text meaning an array of text.
>
> I’m asking because I have had this suspicion for some time. I saw a few
> other procs using the same pattern, for example:
> ```
> { oid => '6501', descr => 'get DDL to recreate a role',
> proname => 'pg_get_role_ddl', prorows => '10', provariadic => 'text',
> proisstrict => 'f', proretset => 't', provolatile => 's',
> pronargdefaults => '1', prorettype => 'text', proargtypes => 'regrole
> text',
> proallargtypes => '{regrole,text}', proargmodes => '{i,v}',
> proargdefaults => '{NULL}', prosrc => 'pg_get_role_ddl' },
> ```
>
> But for jsonb_delete etc procs, _text is used:
> ```
> { oid => '3343',
> proname => 'jsonb_delete', provariadic => 'text', prorettype => 'jsonb',
> proargtypes => 'jsonb _text', proallargtypes => '{jsonb,_text}',
> proargmodes => '{i,v}', proargnames => '{from_json,path_elems}',
> prosrc => 'jsonb_delete_array' },
> ```
>
> So I wonder whether “text” rather than “_text" is intentionally used in
> proallargtypes, or if this was just never noticed.
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
>
>
>
>
| Attachment | Content-Type | Size |
|---|---|---|
| v9-0001-Add-pg_get_table_ddl-to-reconstruct-CREATE-TABLE.patch | application/octet-stream | 134.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nisha Moond | 2026-06-22 12:49:41 | Re: Support EXCEPT for TABLES IN SCHEMA publications |
| Previous Message | Hayato Kuroda (Fujitsu) | 2026-06-22 12:39:47 | RE: Include sequences in publications created by pg_createsubscriber |