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

On Wed, 04 Mar 2026 at 18:29, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)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.