| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Akshay Joshi <akshay(dot)joshi(at)enterprisedb(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 06:54:35 |
| Message-ID: | 983C7B03-D9C3-4F15-8913-7616EC5E6414@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> 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/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Hayato Kuroda (Fujitsu) | 2026-06-22 06:56:45 | RE: doc: should pg_createsubscriber be grouped as a client application? |
| Previous Message | Peter Eisentraut | 2026-06-22 06:39:57 | doc: should pg_createsubscriber be grouped as a client application? |