>> On Wed, Oct 22, 2025 at 12:27 PM Chao Li <[email protected]> wrote:
>>
>> > While reviewing a similar patch of pg_get_policy_ddl(), it take the last 
>> > parameter as a pretty flag. I wonder why pg_get_domain_ddl() doesn’t 
>> > support an argument for pretty?

We have now added pretty printing support in the latest version; see
attached patch. FYI, we tried to stay consistent in the implementation
with pg_get_policy_ddl from
https://www.postgresql.org/message-id/flat/CANxoLDdJsRJqnjMXV3yjsk07Z5iRWxG-c2hZJC7bAKqf8ZXj_A%40mail.gmail.com
or

On Thu, Oct 23, 2025 at 11:20 AM Akshay Joshi
<[email protected]> wrote:
>
>> I've already implemented a generic function for pretty-formatted DDL in the 
>> ruleutils.c file as part of my pg_get_policy_ddl patch. I suggest reusing it 
>> once my patch is accepted and committed by the community.

Thanks Akshay, we adopted your "get_formatted_string()" function into
our path and tried to follow similar implementation patterns as well.

On Thu, Oct 23, 2025 at 6:22 AM jian he <[email protected]> wrote:
>
> I’ve done some refactoring, hope it’s now more intuitive to you.
> Since a domain’s base type can itself be another domain, it’s better to use
>
>     appendStringInfo(&buf, "CREATE DOMAIN %s AS %s",
>                      generate_qualified_type_name(domain_oid),
>                      generate_qualified_type_name(typForm->typbasetype));
>
> then the domain's base type is also fully qualified.

Thanks for the feedback and refactoring Jian! We adopted the
"generate_qualified_type_name" into our patch; this is much better.


> I also refactored the logic for printing domain constraints, which should 
> reduce
> syscache lookups or table scans compared to your version.

we did a lot of refactoring as well while integrating the
pretty-printing support and aligning with e.g. the pg_get_policy_ddl
command. Some of this refactoring follows your suggestiong.
There is one change we decided not to adopt: constructing the
ddl-strings _while_ scanning for constraints in order to optimize the
syscache lookups. The reason is this:

the optimization will save one "SearchSysCache1" per constraint in the
domain. But we still call "pg_get_constraintdef_worker" for each
constraint which does a full table scan.
So in that context, saving the cache lookup seems like a minor
improvement. To us it seemed more desirable to leave the code
unoptimized in this location so that constraint scan and constraint
processing can be decoupled into individual single-purpose
functions/blocks.
Let us know what you think.





Best regards,
Florin Irion
Tim Waizenegger
EDB (EnterpriseDB)

Attachment: v3-0001-Add-pg_get_domain_ddl-function-to-reconstruct-CRE.patch
Description: Binary data

Reply via email to