Re: [PATCH] Add pg_get_database_ddl() function to reconstruct CREATE DATABASE statement - Mailing list pgsql-hackers

On Wed, 19 Nov 2025 at 16:36, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
> On Wed, Nov 19, 2025 at 3:48 PM Japin Li <japinli@hotmail.com> wrote:
>
>  Hi Akshay,
>
>  Thanks for updating the patch.
>
>  On Tue, 18 Nov 2025 at 13:33, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
>  > Hi Chao
>  >
>  > Thanks for reviewing my patch.
>  >
>  > On Tue, Nov 18, 2025 at 5:59 AM Chao Li <li.evan.chao@gmail.com> wrote:
>  >
>  >  Hi Akshay,
>  >
>  >  I just reviewed v3 and got some comments:
>  >
>  >  > On Nov 17, 2025, at 22:34, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
>  >  >
>  >  > All the review comments have been addressed in v3 patch.
>  >
>  >  1 - ruleutils.c
>  >  ```
>  >  +       if (dbForm->datconnlimit != 0)
>  >  +               get_formatted_string(&buf, prettyFlags, 1, "CONNECTION LIMIT = %d",
>  >  +                                                        dbForm->datconnlimit);
>  >  ```
>  >
>  >  I think this is wrong. Default value of CONNECTION_LIMIT is -1 rather than 0. 0 means no connection is allowed,
>  users
>  >  should intentionally set the value, thus 0 should be printed.
>  >
>  >  2 - ruleutils.c
>  >  ```
>  >  +       if (!attrIsNull)
>  >  +               get_formatted_string(&buf, prettyFlags, 1, "ICU_RULES = %s",
>  >  +                                                        quote_identifier(TextDatumGetCString(dbValue)));
>  >  ```
>  >
>  >  ICU_RULES should be omitted if provider is not icu.
>  >
>  >  3 - ruleutils.c
>  >  ```
>  >  +       if (!HeapTupleIsValid(tupleDatabase))
>  >  +               ereport(ERROR,
>  >  +                               errcode(ERRCODE_UNDEFINED_OBJECT),
>  >  +                               errmsg("database with oid %d does not exist", dbOid));
>  >  ```
>  >
>  >  I believe all existing code use %u to format oid. I ever raised the same comment to the other get_xxx_ddl patch.
>  >
>  >  Fixed all above in the attached v4 patch.
>
>  1.
>  Since the STRATEGY and OID parameters are not being reconstructed, should we
>  document this behavior?
>
>  postgres=# CREATE DATABASE mydb WITH STRATEGY file_copy OID 19876 IS_TEMPLATE true;
>  CREATE DATABASE
>  postgres=# SELECT pg_get_database_ddl('mydb', true);
>              pg_get_database_ddl
>  --------------------------------------------
>   CREATE DATABASE mydb                      +
>           WITH OWNER = japin                +
>                   ENCODING = "UTF8"         +
>                   LC_COLLATE = "en_US.UTF-8"+
>                   LC_CTYPE = "en_US.UTF-8"  +
>                   COLLATION_VERSION = "2.39"+
>                   LOCALE_PROVIDER = 'libc'  +
>                   TABLESPACE = pg_default   +
>                   ALLOW_CONNECTIONS = true  +
>                   CONNECTION LIMIT = -1     +
>                   IS_TEMPLATE = true;
>  (1 row)
>
> The FormData_pg_database structure does not expose the database STRATEGY,

Yes, it's not exposed.

> and reconstructing the OID serves no practical
> purpose. If the majority agrees, I can extend the DDL to include OID.

I'm not insisting on reconstructing those parameters; I mean, we can provide
a sentence to describe this behavior.

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