| 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 |
| 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 |