Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement

From: Tim Waizenegger <tim(dot)waizenegger(at)enterprisedb(dot)com>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, jian he <jian(dot)universality(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Florin Irion <florin(dot)irion(at)enterprisedb(dot)com>
Subject: Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Date: 2025-11-10 12:44:36
Message-ID: CAPgqM1XwWz3SWnkBwy+RSZ-XEPq7i=Hb52AA2-b5qXdEg9XNQA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>> On Wed, Oct 22, 2025 at 12:27 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> 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
<akshay(dot)joshi(at)enterprisedb(dot)com> 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 <jian(dot)universality(at)gmail(dot)com> 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 Content-Type Size
v3-0001-Add-pg_get_domain_ddl-function-to-reconstruct-CRE.patch application/octet-stream 39.0 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2025-11-10 13:17:31 Trying out <stdatomic.h>
Previous Message Daniel Gustafsson 2025-11-10 12:22:06 Re: Move SLRU_PAGES_PER_SEGMENT to pg_config_manual.h