public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Akshay Joshi <[email protected]>
Cc: Zsolt Parragi <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Date: Mon, 22 Jun 2026 14:54:35 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CANxoLDejn2JFXF7kVMtcznW6XyessrJ7WToSaQau2GxBdAGSUg@mail.gmail.com>
References: <CANxoLDfjQnhM=E6JSyYo9s9OdjqoN8s_3wE5yL=kaDu_X8j-dA@mail.gmail.com>
<CANxoLDcxmXNHNyPt5v+LaBJVJmyt=j95D7TDE9SepoM7y15t-w@mail.gmail.com>
<CAN4CZFNqf41TCg1ENccq6DAYQpArCbnKUSidHnRKaVMWGj5pBA@mail.gmail.com>
<CANxoLDfL1AxL=k9SsRNReKQ-sJUSy6RvCqeog5HWckFxff=0Pg@mail.gmail.com>
<CAN4CZFP5-fAGwxMCyKGePgp3N8NCpoFXkruO=5xjTC=gPwqYxw@mail.gmail.com>
<CANxoLDc6UFMsyzbXKF8SpMoXdoKE=-yRPqtvyBMKKJ2zOUKX4A@mail.gmail.com>
<CAN4CZFMhnZyQsdLC5iZNego77JHJyP6J_4CpWu=99vKHCd-vzg@mail.gmail.com>
<CANxoLDco7z7Fj7_pj_3YQLvjq8ZH2Ai4c3GcOamed01fZhJ77Q@mail.gmail.com>
<CAB-JLwY=RYPJhdFid0YDqk1L1OQPuHFz9J2tOeeVCFLjov+Ktg@mail.gmail.com>
<CANxoLDdRts-F+QxL+wtM8529Z3+TUJku8PKy3WoxcWBs7zQJ-w@mail.gmail.com>
<CAB-JLwbyzF+JRPCoLCf-H6dQ_1+-O_wBZcESRdC6_H2z+-d-gg@mail.gmail.com>
<CANxoLDcWwoWOxvmV_uBBUsbGf+_AZzBV1GkcjoX+omNpcrCNMQ@mail.gmail.com>
<CAN4CZFM99cGopPJXD_t05+Y9CnocOOTipfXCzoAfBUFSC-i4+A@mail.gmail.com>
<CANxoLDejn2JFXF7kVMtcznW6XyessrJ7WToSaQau2GxBdAGSUg@mail.gmail.com>
> On Jun 22, 2026, at 14:26, Akshay Joshi <[email protected]> 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 <[email protected]> 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/
view thread (54+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox