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

On Thu, 5 Mar 2026 at 08:16, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

>
>
> On Thu, Mar 5, 2026 at 8:15 PM Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>
> wrote:
>
>> I like the idea for this patch and find this useful.
>>
>> I have few comments for the patch, looking at the test cases added in the
>> patch, it is not clear to me what is the default value for each of the
>> options. For example, in
>>
>
> Following the postgresql documentation, I created ddl_defaults.h to
> handle default options. This will be used across all database objects in
> future pg_get_<object>_ddl patches.
>
>
>> +-- With No Tablespace
>> +SELECT ddl_filter(pg_get_database_ddl('regression_utf8', 'defaults',
>> true, 'tablespace', false));
>> +
>> ddl_filter
>>
>>
>> +----------------------------------------------------------------------------------------------------------------------------------------------------------
>> + CREATE DATABASE regression_utf8 WITH OWNER = regress_datdba_after
>> ENCODING = 'UTF8' ALLOW_CONNECTIONS = true CONNECTION LIMIT = 123
>> IS_TEMPLATE = false;
>> +(1 row)
>> +
>> owner option is omitted but the output contains the owner information. So
>> does it mean that the default value for each option is true?
>> But that doesn't seem so, because omitting pretty option, doesn't give
>> output in pretty, rather pretty is used only when provided.
>> I find this rather confusing, it is worth documenting what is the default
>> value for each of these options.
>>
>
> In my view, every database has an owner; it's a required attribute,
> not an optional one with a "default" value. The OWNER clause is correctly
> controlled only by the PG_DDL_NO_OWNER flag. Users must explicitly provide (*'owner',
> false/'no'/'off')* to opt out of including ownership info in the
> DDL. When comparing the Owner and Pretty options: by default, the Owner is
> included in the DDL reconstruction, whereas the Pretty option is set to
> false by default.
>
> Right, I think this needs to be added in the documentation as part of this
patch. Basically, here
<para>
+ <literal>'pretty', true</literal> (or <literal>'pretty',
'yes'</literal>):
+ Adds newlines and indentation for better readability.
+ </para>
it needs to have a line more saying, by default this option is off.
Similarly for other options, like for the owner it is by default true, etc.

> In my initial patches, I used flags like --no-owner and
> --no-tablespace because they are very intuitive for users. However,
> reviewers noted that this style is unique to pg_dump and not used
> elsewhere. Consequently, I switched to the existing name-value pair format
> using VARIADIC arguments.
>
>>
>> Similarly, what is the expected behaviour when defaults option is not
>> provided,
>> +-- With No Owner
>> +SELECT ddl_filter(pg_get_database_ddl('regression_utf8', 'owner',
>> false));
>> + ddl_filter
>> +--------------------------------------------------------------
>> + CREATE DATABASE regression_utf8 WITH CONNECTION LIMIT = 123;
>> +(1 row)
>> +
>> +-- With No Tablespace
>> +SELECT ddl_filter(pg_get_database_ddl('regression_utf8', 'defaults',
>> true, 'tablespace', false));
>> +
>> ddl_filter
>>
>>
>> +----------------------------------------------------------------------------------------------------------------------------------------------------------
>> + CREATE DATABASE regression_utf8 WITH OWNER = regress_datdba_after
>> ENCODING = 'UTF8' ALLOW_CONNECTIONS = true CONNECTION LIMIT = 123
>> IS_TEMPLATE = false;
>> +(1 row)
>> Why is connection_limit present in both of these cases...?
>>
>> Another thing, what is the significance of having defaults option,
>> because I think that knowing non-default values could be more useful, or
>> atleast there should be a way to know the non-default options for the
>> database.
>>
>
> The defaults option is false by default; if a user does not specify
> it, the reconstructed DDL only includes non-default parameters. In my
> opinion, we should retain this flag for users who wish to see all available
> parameters for easier manual editing later. While simple database objects
> have fewer parameters, this flag is essential for complex TABLE or FUNCTION
> syntax where the parameter list is extensive.
>
This information should also be included in the documentation.

>
>
>> Also, the option strategy is missing in the output, is it deliberate? If
>> yes, why?
>>
>
>> On Thu, 5 Mar 2026 at 01:50, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
>> wrote:
>>
>>> 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 <japinli(at)hotmail(dot)com> wrote:
>>>
>>>> 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.
>>>>
>>>
>>
>> --
>> Regards,
>> Rafia Sabih
>> CYBERTEC PostgreSQL International GmbH
>>
>

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH