public inbox for [email protected]  
help / color / mirror / Atom feed
From: 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