Re: pg_get__*_ddl consolidation

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.