From: Japin Li <japinli@hotmail.com>
To: Andrew Dunstan <andrew@dunslane.net>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_get__*_ddl consolidation
In-Reply-To: <e247c261-e3fb-4810-81e0-a65893170e94@dunslane.net> (Andrew
	Dunstan's message of "Thu, 19 Mar 2026 14:34:22 -0400")
References: <e247c261-e3fb-4810-81e0-a65893170e94@dunslane.net>
User-Agent: mu4e 1.12.12; emacs 29.3
Date: Sat, 21 Mar 2026 00:38:44 +0800
Message-ID: 
 <SY7PR01MB10921EFEC3F7CAC3370CC750FB64CA@SY7PR01MB10921.ausprd01.prod.outlook.com>
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
X-MS-PublicTrafficType: Email
X-MS-TrafficTypeDiagnostic: SY7PR01MB10921:EE_|ME3PR01MB7063:EE_
X-MS-Office365-Filtering-Correlation-Id: a8ba2ff9-ad18-4d0a-a8bf-08de869f29b5
X-Microsoft-Antispam: 
	BCL:0;ARA:14566002|461199028|8060799015|23021999003|19110799012|15080799012|41001999006|24121999003|22091999003|5072599009|56899033|1602099012|40105399003|52005399003|4302099013|440099028|10035399007|3412199025;
X-Microsoft-Antispam-Message-Info: 
	=?us-ascii?Q?npYPHQ3VQwKRGBIDAiAO7nh+cObMftDprp/xHbX9NVOdadxKgoCiIHqA9IJd?=
 =?us-ascii?Q?sJyNWr5JYkm8NMmuq7n+NU0xdB1PbFxHgeuxtJDRCVuYtppV+Vx26t/rD8W5?=
 =?us-ascii?Q?0ev/747ZNMlWH4LFt7ZId+2yitQfD4yM3uvULTEGYCxjHLOoIlAp6Lsq7nwV?=
 =?us-ascii?Q?ZLS3RH9QzHapAR0+3xtqPoUnT2BWtelPSJbb+G7vGEtJlvF25PHpaHhElSN9?=
 =?us-ascii?Q?CX+/6kv0x9KkhH45Ea+Rk7+5nsrpXarj1R6lVrSzsb0KqyYh4wIOoze9QHiQ?=
 =?us-ascii?Q?feGZRQgKiuLXtcso79OavK4jLEzfDYju27spa46+ChkCpWZJivRVU1hlwLWD?=
 =?us-ascii?Q?HIVr/VQpuTkxYMdRfjaoRv3zlsr5qmyW9I7OScN3/DLx/1csPs0kA5oNRyeC?=
 =?us-ascii?Q?CLzU/6FVdCxJ8eh3C2xQ+uefz1sSQJDipJ+lT26f5Da/3E4ZbblHj/yZ01V3?=
 =?us-ascii?Q?1BrXeHJYC83A7b3WyDKZCZonUh9QkL++Z3h2gSRRB9bw09LOl/bg2KaBuXcc?=
 =?us-ascii?Q?1GPLWhkVCfQSZQ3yRnHvhTFmtNqipEJavi+MK+fHiBLm/capozHhfDOUJUPb?=
 =?us-ascii?Q?L3ZQrLksg0zM3nscdmfLB76G4cnsoCGXFbr19lhZIRo0cOOjOeGiUot0Azy0?=
 =?us-ascii?Q?LY4kCKD0b9Ik8QwNKa+ETSICXDMGho0vR+tKwR0fIRvpjlyCKCD931iFNAbI?=
 =?us-ascii?Q?I1OVesNb584RVpyRGmMa7vVq80AHPvjrnrqRC26Jkn2HHQThPmV0nrdopjfK?=
 =?us-ascii?Q?wy9wBkkrTSGzaijzAFRSpPwroJIyySQImrfQytMwx/Lcmu07c3ZrFrfqR9kD?=
 =?us-ascii?Q?76yUNNbWJgsq7LMiLyOvX0Cm0GXwK3uJjJLZ5lRGqFO2C6qkCnSQRUPTQsaC?=
 =?us-ascii?Q?lmSD0fq/TDpVDbeH4reYIGhsrrmImANoIeaFBc+ypB6XRfccCKKXDxPuI880?=
 =?us-ascii?Q?EGcboKQFl/txiZwvkJMQ1tDKJcb8ovjW4OIVEJC/b/SHT5kgFCc5905YyGTY?=
 =?us-ascii?Q?O1Pu6xacD6KoR9amUs0r5jVS0JL8bR2SKKPbjDPkNmYpGyrgFus9C94g/o9j?=
 =?us-ascii?Q?fiUf//+e7zXuechMUrnKRuNq3URdkQ=3D=3D?=
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?5QxT06y33NeN09szpAn4yAHvTrABe06pvp9fCh3X0+49CVkKRxvKFQPUdsSA?=
 =?us-ascii?Q?UgmWLIhYlYgCVGT3AQjmL9uzrXP7ovFqh/pdiEf63/zACjDfThhsZ7J5pznG?=
 =?us-ascii?Q?FCLBNXcg7MjaAWhNXciNkXIIXBDsFORNPbiQ6nQvtxtyz1+MPSMBD+O1y3py?=
 =?us-ascii?Q?fEFznWP7wAA5JW91MgWBZH0Ci8uI+64SW7u6UVDmFcAKIGagW3iEP+9JKMLJ?=
 =?us-ascii?Q?oY/6JXcB3SD/BIlsY/M7uAiQqa3nElsJQusB8oIkFlSsqgsiM0V8WIgGcwUA?=
 =?us-ascii?Q?r8yrPNK0r2gJev0O2ffukq26qJjilX0xuTdWg9JGrvyNzWg8O4paxymVQYty?=
 =?us-ascii?Q?JAbakQzTfXM0dVflDc2d9EgXtkk7AwPM1UUO13ENhaO9c2PqgIww4beVkzHJ?=
 =?us-ascii?Q?+KGPfM1gn+wxDFR+EQyDNR7eNIoJOeEdAS553/ZhTtvub604xx9oAv4SHxKP?=
 =?us-ascii?Q?3XScsyDSqU6WkQ7yNPxS7oJjXMZ0f5AK0fn/dTzY3SpidJJAphvn0Br9kNcB?=
 =?us-ascii?Q?yVaEIuzsrBpz7VW0xHELhOcYiZElK8BJJZFNM4z/gV6nQFlvMNpQpmJZxQw1?=
 =?us-ascii?Q?Klq0rhLH+c27JQ+Gw6ZQmZBFGDcmit8dwhESzbdysQbEJL5PU3pSrzuWg3pD?=
 =?us-ascii?Q?Uywxz+gt+hj4aXj71buDcASZn3i3sFMMdUAEMCnjgxBNLHnTj8H+DXG9MRYP?=
 =?us-ascii?Q?SFdfBUDLO8fbJGP2IY5qZRzeDm+4oIZZ+Zq8n4QWf6tIjcTmNXOY4XQzced2?=
 =?us-ascii?Q?75rCUkJ2TJhZ6RL4Lv/9Wone6r1iRRaA5gMIgpPMRHxA9PmgyRZr5hY6o1hk?=
 =?us-ascii?Q?2RH+MJXjdk377lFuY4Wwz1mTgtfu4saJZhdr9Vn92J00muCIBH0okKWuy0Qk?=
 =?us-ascii?Q?bFPE8cOQ4AmViAh4NTsUQb7srlGLxi4fXUwIz0nGuyySHE0jTCIVmUHodeex?=
 =?us-ascii?Q?cpdnPythdomux5pfVueH957nLFZKQFdSJhkGNT96HGg0FNEfDa5s4vz08nsp?=
 =?us-ascii?Q?QqV5QitsayhFa9GidXj5zIeP8oYhODJsSCz5uVMucoyBx94Fe+EWWarY+t5Z?=
 =?us-ascii?Q?veBQ2ZxIfU0J8DtSqN4sQSXNvTD6P4lVV5QWbJXiuC+pqoiCKKr+khgzSnFE?=
 =?us-ascii?Q?x9BdIR/AW7znMjTfDehbfU2bNOb0G9wxyUohd+tayB+q7m+prpkXmjoQ0Pj8?=
 =?us-ascii?Q?Qn4AYgwPHCh5CsVgY0K84J4StB41NY70NEnnlBlrn1Iv6dNbEyL6M7omIw5g?=
 =?us-ascii?Q?h7ZaXJA2SypWXOlaTw3Vuit1gPThi1CFGUs77LXzS/B/aHUjlwJQfXjM9jJ7?=
 =?us-ascii?Q?IjdpS8YvGbhoMoG4/dtbmkPycDee84Ks417fjyPekVJPZaDu+WafdAstx919?=
 =?us-ascii?Q?30SIEvEQGXOy2CjZi7sYkBtdMuRH?=
X-OriginatorOrg: sct-15-20-9412-4-msonline-outlook-feddd.templateTenant
X-MS-Exchange-CrossTenant-Network-Message-Id: 
 a8ba2ff9-ad18-4d0a-a8bf-08de869f29b5
X-MS-Exchange-CrossTenant-AuthSource: SY7PR01MB10921.ausprd01.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Mar 2026 16:38:49.4362
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 
	00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: ME3PR01MB7063
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Archived-At: 
 <https://www.postgresql.org/message-id/SY7PR01MB10921EFEC3F7CAC3370CC750FB64CA%40SY7PR01MB10921.ausprd01.prod.outlook.com>
Precedence: bulk

Hi, Andrew

On Thu, 19 Mar 2026 at 14:34, Andrew Dunstan <andrew@dunslane.net> wrote:
> Greetings
>
> Euler Taveira and I have been working on consolidating these patches.
>
> These patches came out of a suggestion from me some time back [1], and
> I used it as the base for some work at an EDB internal
> program. Perhaps I was motivated a bit by Mao's dictum "Let a hundred
> flowers bloom; let a hundred schools of thought contend." I wanted to
> see what people would come up with. Therefore, if this has seemed a
> bit chaotic, I apologize, both to the authors and to the list. I won't
> do things quite this way in future.
>
> Rather than adding to the already huge ruleutils.c, we decided to
> create a new ddlutils.c file to contain these functions and their
> associated infrastructure. There is in fact a fairly clean separation
> between these functions and ruleutils. We just need to expose one
> function in ruleutils.
>
> We (Euler and I) decided to concentrate on setting up common
> infrastucture and ensuring a common argument and result structure. In
> this first round, we are proposing to add functions for getting the
> DDL for databases, tablespaces, and roles. We decided to stop there
> for now. This sets up a good basis for dealing with more object types
> in future. To the authors of the remaining patches - rest assured you
> have not been forgotten.
>
> Patch 1 sets up the functions used by the rest for option parsing. see [2]
> Patch 2 implements pg_get_role_dll see[3]
> Patch 3 implements pg_get_tabespace_ddl see [4]
> Patch 4 implements pg_get_database_ddl see [2]
>

Thanks for updating the patches.  Here are some initial comments.

Patch 1
=======

1.
+       DDL_OPT_INT
+} DdlOptType;

A trailing comma should be added to match our coding conventions.

2.
+       bool            boolval;                /* filled in for DDL_OPT_BOOL */
+       char       *textval;            /* filled in for DDL_OPT_TEXT (palloc'd) */
+       int                     intval;                 /* filled in for DDL_OPT_INT */

Is it feasible to use a union for these three fields?

3.
+append_ddl_option(StringInfo buf, bool pretty, int indent,
+                                 const char *fmt,...)
+{
+       va_list         args;

IMO, limiting the variable scope to the for loop would be better.


Patch 2
=======

1.
+                foreach(lc, namelist)
+                {
+                    char       *curname = (char *) lfirst(lc);
+
+                    appendStringInfoString(&buf, quote_literal_cstr(curname));
+                    if (lnext(namelist, lc))
+                        appendStringInfoString(&buf, ", ");
+                }

We can use a boolean variable, such as first, to avoid calling lnext(),
and then replace foreach() with foreach_ptr().

2.
+       if (funcctx->call_cntr < funcctx->max_calls)
+       {
+               char       *stmt;
+
+               lc = list_nth_cell(statements, funcctx->call_cntr);
+               stmt = (char *) lfirst(lc);
+
+               SRF_RETURN_NEXT(funcctx, CStringGetTextDatum(stmt));
+       }

Could we use list_nth() in a similar manner to patch 3?


Patch 4
=======

Same as patch 2.

>
> cheers
>
>
> andrew
>
>
> [1]
> https://www.postgresql.org/message-id/flat/945db7c5-be75-45bf-b55b-cb1e56f2e3e9%40dunslane.net
>
> [2]
> https://www.postgresql.org/message-id/flat/CANxoLDc6FHBYJvcgOnZyS+jF0NUo3Lq_83-rttBuJgs9id_UDg@mail.gmail.com
>
> [3]
> https://www.postgresql.org/message-id/flat/4c5f895e-3281-48f8-b943-9228b7da6471@gmail.com
>
> [4]
> https://www.postgresql.org/message-id/flat/CAKWEB6rmnmGKUA87Zmq-s=b3Scsnj02C0kObQjnbL2ajfPWGEw@mail.gmail.com
>
>
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com
>
> [2. text/x-patch; 0001-Add-DDL-option-parsing-infrastructure-for-pg_get_-_d.patch]...
>
> [3. text/x-patch; 0002-Add-pg_get_role_ddl-function.patch]...
>
> [4. text/x-patch; 0004-Add-pg_get_database_ddl-function.patch]...
>
> [5. text/x-patch; 0003-Add-pg_get_tablespace_ddl-function.patch]...

-- 
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.