Re: [PATCH] Add pg_get_database_ddl() function to reconstruct CREATE DATABASE statement

[Akshay Joshi]

I’ve addressed all of your comments.

Attached is the *v11 patch*, now ready for further review.

On Wed, Mar 4, 2026 at 7:31 PM Japin Li <japi...@hotmail.com> wrote:

> On Wed, 04 Mar 2026 at 18:29, Akshay Joshi <akshay.jo...@enterprisedb.com>
> wrote:
> > Thanks for the review, Japin. I’ve addressed all of your comments. I
> also added a check to throw an error if an option
> > appears more than once.
> >
> > Attached is the v10 patch, now ready for further review.
> >
>
> Thanks for updating the patch.  Here are some comments on v10.
>
> 1.
> + * db_oid - OID/Name of the database for which to generate the DDL.
>
> Should the comment be updated? The code only accepts an OID for `db_oid`,
> database names are not supported.
>
> 2.
> +       /* Set the OWNER in the DDL if owner is not omitted */
> +       if (OidIsValid(dbform->datdba) && !(ddl_flags & PG_DDL_NO_OWNER))
> +       {
> +               get_formatted_string(&buf, pretty_flags, 8, "OWNER = %s",
> +
> quote_identifier(dbowner));
> +       }
>
> `dbowner` is only needed inside this `if` — how about declaring it there to
> reduce its scope?
>
> 3.
> +               /* If is_with_defaults is true, then we skip default
> encoding check */
> +               if (is_with_defaults ||
> +
>  (pg_strcasecmp(pg_encoding_to_char(dbform->encoding),
> +
> DDL_DEFAULTS.DATABASE.ENCODING) != 0))
> +               {
> +                       get_formatted_string(&buf, pretty_flags, 8,
> "ENCODING = %s",
> +
> quote_literal_cstr(
> +
>                              pg_encoding_to_char(dbform->encoding)));
> +               }
>
> How about cache the result of `pg_encoding_to_char()` in a local variable
> to
> avoid calling it twice?
>
> --
> Regards,
> Japin Li
> ChengDu WenWu Information Technology Co., Ltd.
>

v11-0001-Add-pg_get_database_ddl-function-to-reconstruct-ddl.patch
Description: Binary data