Thread
-
[PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Tim Waizenegger <tim.waizenegger@enterprisedb.com> — 2025-10-16T09:16:54Z
Hi all, Following the recent "Retail DDL" discussion [1], we're submitting another implementation: pg_get_domain_ddl(). This function reconstructs CREATE DOMAIN statements for existing domains, following what seems to be the agreed pg_get_{objecttype}_ddl naming convention. ## Function pg_get_domain_ddl(regtype) returns text Returns a complete CREATE DOMAIN statement including base type, default values, and all constraints. Uses get_typdefault() for proper expression handling and supports schema-qualified domains. ## Example ``` CREATE DOMAIN regress_us_postal_code AS TEXT DEFAULT '00000' CONSTRAINT regress_us_postal_code_check CHECK ( VALUE ~ '^\d{5}$' OR VALUE ~ '^\d{5}-\d{4}$' ); SELECT pg_get_domain_ddl('regress_us_postal_code'); pg_get_domain_ddl ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- CREATE DOMAIN public.regress_us_postal_code AS text DEFAULT '00000'::text CONSTRAINT regress_us_postal_code_check CHECK (VALUE ~ '^\d{5}$'::text OR VALUE ~ '^\d{5}-\d{4}$'::text); (1 row) ``` ## Implementation - New "Get Object DDL Functions" documentation section - Comprehensive regression tests in a separate file where we will add tests for the other objects functions. We're unsure about the place where to add the trigger to the `object_ddl` test. We added it now in `src/test/regress/parallel_schedule`, please let us know if there is a better place. This is part of a coordinated effort where we've divided the DDL functions among different contributors. Additional patches for other object types (tables, indexes, etc.) will follow from other team members. Already submitted are: CREATE TRIGGER [2] and CREATE POLICY [3]. Patch attached. Feedback welcome. [1] https://www.postgresql.org/message-id/flat/945db7c5-be75-45bf-b55b-cb1e56f2e3e9%40dunslane.net [2] https://www.postgresql.org/message-id/flat/CAPXBC8K5awmtMoq66DGHe%2BnD7hUf6HPRVHLeGNBRpCDpzusOXQ%40mail.gmail.com [3] https://www.postgresql.org/message-id/flat/CANxoLDdJsRJqnjMXV3yjsk07Z5iRWxG-c2hZJC7bAKqf8ZXj_A%40mail.gmail.com --- Best regards, Florin Irion Tim Waizenegger EDB (EnterpriseDB) -
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
jian he <jian.universality@gmail.com> — 2025-10-16T11:04:22Z
On Thu, Oct 16, 2025 at 5:17 PM Tim Waizenegger <tim.waizenegger@enterprisedb.com> wrote: > > Hi all, > > Following the recent "Retail DDL" discussion [1], we're submitting another > implementation: pg_get_domain_ddl(). > > This function reconstructs CREATE DOMAIN statements for existing domains, > following what seems to be the agreed pg_get_{objecttype}_ddl naming convention. > > ## Function > > pg_get_domain_ddl(regtype) returns text > > Returns a complete CREATE DOMAIN statement including base type, default values, > and all constraints. Uses get_typdefault() for proper expression handling and > supports schema-qualified domains. > <indexterm> + <primary>pg_get_domain_ddl</primary> + </indexterm> + <function>pg_get_domain_ddl</function> ( <parameter>domain</parameter> <type>text</type> ) + <returnvalue>text</returnvalue> + </para> + <para> + Reconstructs the creating command for a domain. + The result is a complete <command>CREATE DOMAIN</command> statement. + </para></entry> <type>text</type> should be <type>regtype</type> + Oid domain_oid = PG_GETARG_OID(0); + HeapTuple typeTuple; ,.... + + /* Look up the domain in pg_type */ + typeTuple = SearchSysCache1(TYPEOID, ObjectIdGetDatum(domain_oid)); + select pg_get_domain_ddl(-1); will cause segfault. see https://www.postgresql.org/message-id/3759807.1711658868%40sss.pgh.pa.us and pg_get_trigger_ddl thread. NOT VALID check constraint handling is tricky currently. create domain x as int; alter domain x add constraint cc check(value > 2) not valid; select pg_get_domain_ddl('x'::regtype); CREATE DOMAIN public.x AS integer CONSTRAINT cc CHECK (VALUE > 2) NOT VALID; but putting the above to psql would result in syntax error. https://www.postgresql.org/docs/current/sql-createdomain.html [ COLLATE collation ] part not handled? create domain d0 as text collate "C"; select pg_get_domain_ddl('d0'::regtype); pg_get_domain_ddl ---------------------------------- CREATE DOMAIN public.d0 AS text; (1 row) we should expect CREATE DOMAIN public.d0 AS text COLLATE "C"; -
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Tim Waizenegger <tim.waizenegger@enterprisedb.com> — 2025-10-22T09:32:25Z
On Thu, Oct 16, 2025 at 1:05 PM jian he <jian.universality@gmail.com> wrote: > > On Thu, Oct 16, 2025 at 5:17 PM Tim Waizenegger > <tim.waizenegger@enterprisedb.com> wrote: > > > > Hi all, > > > > Following the recent "Retail DDL" discussion [1], we're submitting another > > implementation: pg_get_domain_ddl(). > > > > select pg_get_domain_ddl(-1); > will cause segfault. > see https://www.postgresql.org/message-id/3759807.1711658868%40sss.pgh.pa.us > and pg_get_trigger_ddl thread. > > > NOT VALID check constraint handling is tricky currently. > create domain x as int; > alter domain x add constraint cc check(value > 2) not valid; > > select pg_get_domain_ddl('x'::regtype); > CREATE DOMAIN public.x AS integer CONSTRAINT cc CHECK (VALUE > 2) NOT VALID; > but putting the above to psql would result in syntax error. > > > https://www.postgresql.org/docs/current/sql-createdomain.html > [ COLLATE collation ] > part not handled? > > create domain d0 as text collate "C"; > select pg_get_domain_ddl('d0'::regtype); > pg_get_domain_ddl > ---------------------------------- > CREATE DOMAIN public.d0 AS text; > (1 row) > > we should expect > CREATE DOMAIN public.d0 AS text COLLATE "C"; Thanks for the feedback! We addressed the issues mentioned above and also added more extensive test cases: postgres=# select pg_get_domain_ddl(-1); pg_get_domain_ddl ------------------- (1 row) postgres=# create domain d0 as text collate "C"; CREATE DOMAIN postgres=# select pg_get_domain_ddl('d0'::regtype); pg_get_domain_ddl ---------------------------------------------- CREATE DOMAIN public.d0 AS text COLLATE "C"; (1 row) postgres=# create domain x as int; CREATE DOMAIN postgres=# alter domain x add constraint cc check(value > 2) not valid; ALTER DOMAIN postgres=# select pg_get_domain_ddl('x'::regtype); pg_get_domain_ddl ---------------------------------------------------------------------- CREATE DOMAIN public.x AS integer; + ALTER DOMAIN public.x ADD CONSTRAINT cc CHECK (VALUE > 2) NOT VALID; (1 row) updated patch is attached --- Best regards, Florin Irion Tim Waizenegger EDB (EnterpriseDB) -
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Chao Li <li.evan.chao@gmail.com> — 2025-10-22T10:26:44Z
Hi Tim, Thanks for working on this. I haven’t finished reviewing the entire patch. But I got a quick question: > On Oct 22, 2025, at 17:32, Tim Waizenegger <tim.waizenegger@enterprisedb.com> wrote: > > updated patch is attached > > --- > Best regards, > Florin Irion > Tim Waizenegger > > EDB (EnterpriseDB) > <v1-0001-Add-pg_get_domain_ddl-function-to-reconstruct-CRE.patch> ``` +/* + * pg_get_domain_ddl - Get CREATE DOMAIN statement for a domain + */ +Datum +pg_get_domain_ddl(PG_FUNCTION_ARGS) +{ + StringInfoData buf; + Oid domain_oid = PG_GETARG_OID(0); + HeapTuple typeTuple; + Form_pg_type typForm; + Node *defaultExpr; ``` 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? See the code snippet from the other patch: ``` +/* + * pg_get_policy_ddl + * + * Generate a CREATE POLICY statement for the specified policy. + * + * tableID - Table ID of the policy. + * policyName - Name of the policy for which to generate the DDL. + * pretty - If true, format the DDL with indentation and line breaks. + */ +Datum +pg_get_policy_ddl(PG_FUNCTION_ARGS) +{ + Oid tableID = PG_GETARG_OID(0); + Name policyName = PG_GETARG_NAME(1); + bool pretty = PG_GETARG_BOOL(2); # <====== This is the pretty arg + bool attrIsNull; ``` Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ -
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Tim Waizenegger <tim.waizenegger@enterprisedb.com> — 2025-10-22T12:00:21Z
On Wed, Oct 22, 2025 at 12:27 PM Chao Li <li.evan.chao@gmail.com> wrote: > > Hi Tim, > > Thanks for working on this. I haven’t finished reviewing the entire patch. But I got a quick question: > > 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? > > That's a good point; we'll add pretty printing support for consistency with the other functions. I'll send a new patch in the coming days. Best regards, Florin Irion Tim Waizenegger EDB (EnterpriseDB)
-
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
jian he <jian.universality@gmail.com> — 2025-10-23T04:21:34Z
On Wed, Oct 22, 2025 at 5:32 PM Tim Waizenegger <tim.waizenegger@enterprisedb.com> wrote: > > updated patch is attached > 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. I also refactored the logic for printing domain constraints, which should reduce syscache lookups or table scans compared to your version. please check the attached. -
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2025-10-23T09:19:48Z
On Wed, 22 Oct, 2025, 17:30 Tim Waizenegger, < tim.waizenegger@enterprisedb.com> wrote: > On Wed, Oct 22, 2025 at 12:27 PM Chao Li <li.evan.chao@gmail.com> wrote: > > > > Hi Tim, > > > > Thanks for working on this. I haven’t finished reviewing the entire > patch. But I got a quick question: > > > > 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? > > > > > > That's a good point; we'll add pretty printing support for consistency > with the other functions. I'll send a new patch in the coming days. > 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. > > Best regards, > Florin Irion > Tim Waizenegger > EDB (EnterpriseDB) > > >
-
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Tim Waizenegger <tim.waizenegger@enterprisedb.com> — 2025-11-10T12:44:36Z
>> On Wed, Oct 22, 2025 at 12:27 PM Chao Li <li.evan.chao@gmail.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.joshi@enterprisedb.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.universality@gmail.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)
-
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Florin Irion <irionr@gmail.com> — 2025-11-11T16:14:42Z
Hello, Cirrus-CI was complaining because we don't sort the constraints and thus they were making the test fail because of the random order. Made it sort with `list_sort`and `list_oid_cmp`not sure if that's the best thing to sort them. Check v4 attached. Cheers, Florin Irion Tim Waizenegger EDB (EnterpriseDB)
-
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
zengman <zengman@halodbtech.com> — 2025-11-20T06:55:03Z
Quick correction with an apology: I accidentally created a new thread (https://www.postgresql.org/message-id/tencent_64301BB7627E58CD256CE15F%40qq.com) and submitted the patch there—my apologies for the mix-up! Let’s just continue the discussion here as planned. -- Regrads, Man Zeng
-
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Chao Li <li.evan.chao@gmail.com> — 2025-11-20T08:47:19Z
> On Nov 12, 2025, at 00:14, Florin Irion <irionr@gmail.com> wrote: > > Hello, Cirrus-CI was complaining because we don't sort the constraints and thus > they were making the test fail because of the random order. > Made it sort with `list_sort`and `list_oid_cmp`not sure if that's the best > thing to sort them. > Check v4 attached. > Cheers, > Florin Irion > Tim Waizenegger > EDB (EnterpriseDB) > <v4-0001-Add-pg_get_domain_ddl-function-to-reconstruct-CRE.patch> I just tested v4, and see two problems: ``` evantest=# CREATE DOMAIN public.int AS pg_catalog.int4; CREATE DOMAIN evantest=# SELECT pg_get_domain_ddl('int'); ERROR: cache lookup failed for type 0 evantest=# evantest=# evantest=# SELECT pg_get_domain_ddl('pg_class'); ERROR: cache lookup failed for type 0 evantest=# evantest=# evantest=# SELECT pg_get_domain_ddl('public.int'); pg_get_domain_ddl ------------------------------------------------ CREATE DOMAIN public."int" AS pg_catalog.int4; (1 row) evantest=# show search_path; search_path ----------------- "$user", public (1 row) ``` 1. The error message "cache lookup failed for type 0” looks not good. At lease saying something like “domain ‘int’ does not exist”. 2. I created a domain “int” in “public”, as you see, “public” is in the search_path, but SELECT pg_get_domain_ddl('int’); failed. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ -
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Neil Chen <carpenter.nail.cz@gmail.com> — 2025-11-20T09:44:07Z
Hi Florin, +pg_get_domain_ddl_ext(PG_FUNCTION_ARGS) > +{ > + Oid domain_oid = PG_GETARG_OID(0); > + bool pretty = PG_GETARG_BOOL(1); > + char *res; > + int prettyFlags; > + > + prettyFlags = pretty ? GET_PRETTY_FLAGS(pretty) : 0; Seems like we should directly use GET_PRETTY_FLAGS here, as it already checks the value of "pretty". For a "display-oriented" result, using PRETTYFLAG_INDENT looks more appropriate. + appendStringInfo(buf, "CREATE DOMAIN %s AS %s", > + generate_qualified_type_name(typForm->oid), > + generate_qualified_type_name(typForm->typbasetype)); It might be good to first call get_typtype to check if it is TYPTYPE_DOMAIN. -
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Florin Irion <irionr@gmail.com> — 2025-12-02T21:11:39Z
Hello, On 20/11/25 07:55, Man Zeng wrote: > Quick correction with an apology: I accidentally created a new thread (https://www.postgresql.org/message-id/tencent_64301BB7627E58CD256CE15F%40qq.com) and submitted the patch there—my apologies for the mix-up! Let’s just continue the discussion here as planned. On 20/11/25 09:47, Chao Li wrote: > 1. The error message "cache lookup failed for type 0” looks not good. At lease saying something like “domain ‘int’ does not exist”. > > 2. I created a domain “int” in “public”, as you see, “public” is in the search_path, but SELECT pg_get_domain_ddl('int’); failed. Thank you both Man Zeng and Chao Li for checking this. Changes added in v5. I don't think there is a way to make the path issue work, so we just give more info to the caller. We exit with error when a built-in name is used and we throw also a hint saying that schema-qualified domain name should be used to be sure it's not conflicting with a built in name. On 20/11/25 10:44, Neil Chen wrote: > Hi Florin, > > +pg_get_domain_ddl_ext(PG_FUNCTION_ARGS) > +{ > + Oid domain_oid = PG_GETARG_OID(0); > + bool pretty = PG_GETARG_BOOL(1); > + char *res; > + int prettyFlags; > + > + prettyFlags = pretty ? GET_PRETTY_FLAGS(pretty) : 0; > > > Seems like we should directly use GET_PRETTY_FLAGS here, as it already > checks the value of "pretty". For a "display-oriented" result, using > PRETTYFLAG_INDENT looks more appropriate. Well, actually no, GET_PRETTY_FLAGS(false) returns PRETTYFLAG_INDENT But we actually want 0 when pretty is false (no indentation, just spaces) > + appendStringInfo(buf, "CREATE DOMAIN %s AS %s", > + generate_qualified_type_name(typForm->oid), > + generate_qualified_type_name(typForm->typbasetype)); > > > It might be good to first call get_typtype to check if it is > TYPTYPE_DOMAIN. I added this in `pg_get_domain_ddl_worker`, as we need to make this check ASAP. Cheers, Florin Irion Tim Waizenegger EDB (EnterpriseDB) -
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Haritabh Gupta <haritabh1992@gmail.com> — 2026-01-27T18:27:15Z
Hello, Please find below some comments (mostly minor ones): 1. We need to add the following comma in the docs change. so that it looks same as other functions: diff --git a/doc/src/sgml/func/func-info.sgml b/doc/src/sgml/func/func-info.sgml index 25f87b78344..bc01c73f4ea 100644 --- a/doc/src/sgml/func/func-info.sgml +++ b/doc/src/sgml/func/func-info.sgml @@ -3861,7 +3861,7 @@ acl | {postgres=arwdDxtm/postgres,foo=r/postgres} <primary>pg_get_domain_ddl</primary> </indexterm> <function>pg_get_domain_ddl</function> ( <parameter>domain</parameter> <type>regtype</type> - <optional> <parameter>pretty</parameter> <type>boolean</type> </optional>) + <optional>, <parameter>pretty</parameter> <type>boolean</type> </optional>) <returnvalue>text</returnvalue> </para> <para> 2. In the function signature there is `int prettyFlags` argument, while the doc suggests `pretty`: +/* + * get_formatted_string + * + * Return a formatted version of the string. + * + * pretty - If pretty is true, the output includes tabs (\t) and newlines (\n). + * noOfTabChars - indent with specified no of tabs. + * fmt - printf-style format string used by appendStringInfoVA. + */ +static void +get_formatted_string(StringInfo buf, int prettyFlags, int noOfTabChars, const char *fmt,...) 3. In a similar patch (https://www.postgresql.org/message-id/flat/CANxoLDdJsRJqnjMXV3yjsk07Z5iRWxG-c2hZJC7bAKqf8ZXj_A@mail.gmail.com), author has defined a separate macro to make the usage of `GET_PRETTY_FLAGS` cleaner, We can use the same in function `pg_get_domain_ddl_ext`: +#define GET_DDL_PRETTY_FLAGS(pretty) \ + ((pretty) ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) \ + : 0) +Datum +pg_get_policy_ddl(PG_FUNCTION_ARGS) +{ + Oid tableID = PG_GETARG_OID(0); + Name policyName = PG_GETARG_NAME(1); + bool pretty = PG_GETARG_BOOL(2); + int prettyFlags; + char *res; + + prettyFlags = GET_DDL_PRETTY_FLAGS(pretty); 4. Usually the tests for the function to get the DDL definition of an object are present in the same testcase file where the `CREATE...` command exists, e.g. test for `pg_get_indexdef` exists in `create_index.sql` file. Similarly tests for `pg_get_functiondef` exists in `create_procedure.sql` file and so on. Currently in the patch, the tests for `pg_get_domain_ddl` are put in a new file `object_ddl.sql` but I guess it can be put in the existing file `domain.sql` because that is where the `CREATE DOMAIN...` tests reside. -
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Florin Irion <irionr@gmail.com> — 2026-01-29T16:45:52Z
On 27/01/26 19:27, Haritabh Gupta wrote: > Hello, > Please find below some comments (mostly minor ones): > > 1. We need to add the following comma in the docs change. so that it looks same as other functions: > diff --git a/doc/src/sgml/func/func-info.sgml b/doc/src/sgml/func/func-info.sgml > index 25f87b78344..bc01c73f4ea 100644 > --- a/doc/src/sgml/func/func-info.sgml > +++ b/doc/src/sgml/func/func-info.sgml > @@ -3861,7 +3861,7 @@ acl | {postgres=arwdDxtm/postgres,foo=r/postgres} > <primary>pg_get_domain_ddl</primary> > </indexterm> > <function>pg_get_domain_ddl</function> ( <parameter>domain</parameter> <type>regtype</type> > - <optional> <parameter>pretty</parameter> <type>boolean</type> </optional>) > + <optional>, <parameter>pretty</parameter> <type>boolean</type> </optional>) > <returnvalue>text</returnvalue> > </para> > <para> > > > 2. In the function signature there is `int prettyFlags` argument, while the doc suggests `pretty`: > +/* > + * get_formatted_string > + * > + * Return a formatted version of the string. > + * > + * pretty - If pretty is true, the output includes tabs (\t) and newlines (\n). > + * noOfTabChars - indent with specified no of tabs. > + * fmt - printf-style format string used by appendStringInfoVA. > + */ > +static void > +get_formatted_string(StringInfo buf, int prettyFlags, int noOfTabChars, const char *fmt,...) > > > 3. In a similar patch (https://www.postgresql.org/message-id/flat/CANxoLDdJsRJqnjMXV3yjsk07Z5iRWxG-c2hZJC7bAKqf8ZXj_A@mail.gmail.com), author has defined a separate macro to make the usage of `GET_PRETTY_FLAGS` cleaner, We can use the same in function `pg_get_domain_ddl_ext`: > +#define GET_DDL_PRETTY_FLAGS(pretty) \ > + ((pretty) ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) \ > + : 0) > > +Datum > +pg_get_policy_ddl(PG_FUNCTION_ARGS) > +{ > + Oid tableID = PG_GETARG_OID(0); > + Name policyName = PG_GETARG_NAME(1); > + bool pretty = PG_GETARG_BOOL(2); > + int prettyFlags; > + char *res; > + > + prettyFlags = GET_DDL_PRETTY_FLAGS(pretty); Thanks for reviewing! I addressed your first 3 suggestions and attached v6. > 4. Usually the tests for the function to get the DDL definition of an object are present in the same testcase file where the `CREATE...` command exists, e.g. test for `pg_get_indexdef` exists in `create_index.sql` file. Similarly tests for `pg_get_functiondef` exists in `create_procedure.sql` file and so on. Currently in the patch, the tests for `pg_get_domain_ddl` are put in a new file `object_ddl.sql` but I guess it can be put in the existing file `domain.sql` because that is where the `CREATE DOMAIN...` tests reside. For number 4 I think it's better to keep it in a separate file as this is just one of the "get_object_ddl" functions, and in this `object_ddl` file we can add more test also for other functions similar to this one. What do you/others think? Cheers, Florin Irion EDB -- www.enterprisedb.com -
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Florin Irion <irionr@gmail.com> — 2026-02-12T10:11:55Z
Hello, New patch rebased on current master attached. Cheers, -- * Florin Irion * * https://www.enterprisedb.com <https://www.enterprisedb.com/>* -
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Haritabh Gupta <haritabh1992@gmail.com> — 2026-02-18T23:50:16Z
Hi Florin, Thanks for addressing the comments. I tested v7 and found that type modifiers (typmod) are lost in the base type output. In build_create_domain_statement: + appendStringInfo(buf, "CREATE DOMAIN %s AS %s", + generate_qualified_type_name(typForm->oid), + generate_qualified_type_name(typForm->typbasetype)); generate_qualified_type_name does not include the type modifier, so domains over types like varchar(N), numeric(P,S), char(N), bit(N), time(N) etc. silently lose their modifiers. The generated DDL does not roundtrip correctly. create domain d1 as varchar(100); select pg_get_domain_ddl('d1'); pg_get_domain_ddl ---------------------------------------------------------- CREATE DOMAIN public.d1 AS pg_catalog."varchar"; (1 row) we should expect AS character varying(100). Roundtrip confirms the semantic change: ``` select length(repeat('x', 150)::d1); -- returns 100 (truncated) drop domain d1; -- re-execute generated DDL create domain public.d1 as pg_catalog."varchar"; select length(repeat('x', 150)::d1); -- returns 150 (not truncated) ``` Same issue with numeric(15,2): rounds to 2 decimals before roundtrip, full precision after. Also confirmed with char(1), bit(8), time(3), varbit(256). I think for the base type we could use format_type_extended with both FORMAT_TYPE_TYPEMOD_GIVEN and FORMAT_TYPE_FORCE_QUALIFY: appendStringInfo(buf, "CREATE DOMAIN %s AS %s", generate_qualified_type_name(typForm->oid), format_type_extended(typForm->typbasetype, typForm->typtypmod, FORMAT_TYPE_TYPEMOD_GIVEN | FORMAT_TYPE_FORCE_QUALIFY)); Regards, Haritabh -
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Tom Lane <tgl@sss.pgh.pa.us> — 2026-02-19T00:10:19Z
Haritabh Gupta <haritabh1992@gmail.com> writes: > Thanks for addressing the comments. I tested v7 and found that > type modifiers (typmod) are lost in the base type output. This report crystallized something that's been bothering me about not only pg_get_domain_ddl() but all the similar patches that are in the queue. They are adding a large amount of new code that will have to be kept in sync with behavior elsewhere, and there is basically zero forcing function to ensure that that happens. Even the rather-overly-voluminous test cases proposed for the functions cannot catch errors of omission, especially not future errors of omission. I don't really know what to do about this, but I don't like the implementation approach that's being proposed. I think it's loading too much development effort and future maintenance effort onto us in comparison to the expected benefit of having these functions. regards, tom lane
-
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Haritabh Gupta <haritabh1992@gmail.com> — 2026-02-19T00:40:20Z
Hi, Tom Lane <tgl@sss.pgh.pa.us> writes: > They are adding a large amount of new code that will have to be > kept in sync with behavior elsewhere, and there is basically zero > forcing function to ensure that that happens. Agree. For the sake of completeness I did a thorough pass over the rest of v7 and found a few more issues. Documenting them here so they're on the record regardless of where the broader discussion about the approach lands. 1) get_formatted_string silently drops large formatted strings + va_start(args, fmt); + appendStringInfoVA(buf, fmt, args); + va_end(args); appendStringInfoVA returns non-zero when the buffer is too small, requiring enlargeStringInfo + retry (see appendStringInfo in stringinfo.c). The return value is ignored here, so large formatted text is silently lost. Reproduction -- a domain with a ~2647-char CHECK expression: DO $$ DECLARE long_check text; BEGIN long_check := 'CHECK ('; FOR i IN 1..50 LOOP IF i > 1 THEN long_check := long_check || ' OR '; END IF; long_check := long_check || format( 'VALUE ~ ''^pattern_%s_[a-zA-Z0-9]{10,20}$''', i); END LOOP; long_check := long_check || ')'; EXECUTE format( 'CREATE DOMAIN huge_domain AS text CONSTRAINT big_check %s', long_check); END $$; select pg_get_domain_ddl('huge_domain'); CREATE DOMAIN public.huge_domain AS pg_catalog.text CONSTRAINT big_check ; (1 row) The entire CHECK clause (~2647 chars) is silently dropped. This function was adopted from the pg_get_policy_ddl patch [1]. I checked v8 there and confirmed the same bug exists. 2) Function is VOLATILE PARALLEL UNSAFE pg_proc.dat is missing provolatile => 's', and system_functions.sql does not specify STABLE PARALLEL SAFE. Every other pg_get_*def function is STABLE PARALLEL SAFE: select proname, provolatile, proparallel from pg_proc where proname in ('pg_get_domain_ddl','pg_get_constraintdef', 'pg_get_functiondef','pg_get_triggerdef'); pg_get_constraintdef | s | s pg_get_domain_ddl | v | u <-- pg_get_functiondef | s | s pg_get_triggerdef | s | s Same issue in pg_get_policy_ddl v8 [1]. 3) Internal type names exposed (related to typmod bug) generate_qualified_type_name also uses the raw pg_type.typname, so beyond losing modifiers: int[] -> pg_catalog._int4 (should be integer[]) char(5) -> pg_catalog.bpchar (should be character(5)) timestamp(6) -> pg_catalog."timestamp" (should be timestamp(6) without time zone) The format_type_extended fix from my earlier message resolves this too. Several test expectations in object_ddl.out will need updating once fixed. Regards, Haritabh [1] https://www.postgresql.org/message-id/flat/CANxoLDdJsRJqnjMXV3yjsk07Z5iRWxG-c2hZJC7bAKqf8ZXj_A%40mail.gmail.com -
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Andrew Dunstan <andrew@dunslane.net> — 2026-02-20T13:24:25Z
On 2026-02-18 We 7:10 PM, Tom Lane wrote: > Haritabh Gupta <haritabh1992@gmail.com> writes: >> Thanks for addressing the comments. I tested v7 and found that >> type modifiers (typmod) are lost in the base type output. > This report crystallized something that's been bothering me > about not only pg_get_domain_ddl() but all the similar patches > that are in the queue. They are adding a large amount of new > code that will have to be kept in sync with behavior elsewhere, > and there is basically zero forcing function to ensure that > that happens. Even the rather-overly-voluminous test cases > proposed for the functions cannot catch errors of omission, > especially not future errors of omission. > > I don't really know what to do about this, but I don't like the > implementation approach that's being proposed. I think it's > loading too much development effort and future maintenance effort > onto us in comparison to the expected benefit of having these > functions. Do you have an alternative suggestion? We could create an extension, but keeping that in sync might in fact be harder, and we know from experience that extensions are not universally available. That would make leveraging these functions for something like Matheus Alcantara's schema cloning proposal (as I think Alvaro suggested) pretty much impossible. I'm not sure how much maintenance effort you think will be needed. We don't change the shape of database objects all that often. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
-
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Florin Irion <irionr@gmail.com> — 2026-03-02T16:41:41Z
Hi Haritabh, Tom, Thanks for the thorough review. v8 fixes all reported bugs and adds round-trip tests to address the forcing-function concern. Haritabh's bugs — all fixed in attached v8. On 19/02/26 01:10, Tom Lane wrote: > This report crystallized something that's been bothering me > about not only pg_get_domain_ddl() but all the similar patches > that are in the queue. They are adding a large amount of new > code that will have to be kept in sync with behavior elsewhere > and there is basically zero forcing function to ensure that > that happens. Even the rather-overly-voluminous test cases > proposed for the functions cannot catch errors of omission, > especially not future errors of omission. v8 adds a PL/pgSQL round-trip harness that captures DDL, drops the domain, re-executes the DDL, and ASSERTs the regenerated DDL is identical, any suggestions on how to improve it are welcomed. This function can be re-used also with other get_<object>_ddl as it accepts a parameter for the <object_type>, this way we can use some common code. > I don't really know what to do about this, but I don't like the > implementation approach that's being proposed. I think it's > loading too much development effort and future maintenance effort > onto us in comparison to the expected benefit of having these > functions. I understand your point that there are multiple implementations and each have its own way of doing it. I think we should start somewhere and eventually ask further implementations to adapt to use common code or make it work with all existing (at that point) implementations, one at a time. pg_get_domain_ddl implementation uses mainly common code in ruleutils.c plus some glue code. We could maybe also create a new separate module and put all the code for all these features there. What do you think? Cheers, Florin -- *Florin Irion* www.enterprisedb.com <https://www.enterprisedb.com>
-
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Álvaro Herrera <alvherre@kurilemu.de> — 2026-03-02T17:49:59Z
On 2026-Mar-02, Florin Irion wrote: > On 19/02/26 01:10, Tom Lane wrote: > > > This report crystallized something that's been bothering me > > about not only pg_get_domain_ddl() but all the similar patches > > that are in the queue. They are adding a large amount of new > > code that will have to be kept in sync with behavior elsewhere > > and there is basically zero forcing function to ensure that > > that happens. Even the rather-overly-voluminous test cases > > proposed for the functions cannot catch errors of omission, > > especially not future errors of omission. > > v8 adds a PL/pgSQL round-trip harness that captures DDL, drops > the domain, re-executes the DDL, and ASSERTs the regenerated DDL > is identical, any suggestions on how to improve it are welcomed. > This function can be re-used also with other get_<object>_ddl > as it accepts a parameter for the <object_type>, this way we can > use some common code. Hmm, I think this is generally a useful direction, but it feels incomplete because the developer of some new DDL feature has to remember to add calls to the regress_verify_ddl_roundtrip() function in the tests somewhere. We _are_ going to forget. I think it would be more helpful to have a test module that 1. installs an event trigger on ddl_command_end for CREATE for object being created 2. runs all the tests in parallel_schedule 3. do [... something ...] with the event trigger to generate the DDL using the new functions, and compare with the object created originally. (There's a lot of handwaving here. Maybe pg_dump both and compare?) With this sort of approach, the developer of a new CREATE feature doesn't have to remember anything, because that test will break as soon as they add a test to the stock regression tests when they add some new feature that the pg_get_blah_ddl() doesn't support. The whole point is that adding something to the standard regression tests (which is something we're accustomed to doing) is enough to also cover the DDL-dumping functions. Another possibility is to use the pg_dump/t/002_pg_dump.pl database instead of the stock regression one, which is perhaps richer in object type diversity. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "Fundamental layering violations tend to bite you on tender parts of your anatomy." (Tom Lane) https://postgr.es/m/2818249.1748706121@sss.pgh.pa.us
-
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Tom Lane <tgl@sss.pgh.pa.us> — 2026-03-02T18:56:22Z
=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes: > I think it would be more helpful to have a test module that > 1. installs an event trigger on ddl_command_end for CREATE for > object being created > 2. runs all the tests in parallel_schedule > 3. do [... something ...] with the event trigger to generate the DDL > using the new functions, and compare with the object created > originally. (There's a lot of handwaving here. Maybe pg_dump both > and compare?) While I agree that automating this might be helpful, please please please do not create yet another execution of the core regression tests. There is far too much stuff in there that is not DDL and will only be useless cycles for this purpose. I wonder if it'd be practical to extract just the DDL commands from the core scripts, and then run just those through a process like you suggest? I agree that the "handwaving" part is trickier than it looks. If memory serves, we've had bugs-of-omission where somebody forgot to update pg_dump for some new feature, and it wasn't obvious because comparing pg_dump output against pg_dump output didn't show that the relevant object property wasn't copied correctly. In this context, forgetting to update both pg_dump and the DDL-dumping function would mask both omissions. Maybe that's unlikely, but ... > Another possibility is to use the pg_dump/t/002_pg_dump.pl database > instead of the stock regression one, which is perhaps richer in object > type diversity. I think that test script also suffers from the out-of-sight, out-of-mind problem. Not to mention that you need a lot of study to figure out how to modify it at all. I certainly avoid doing so. regards, tom lane
-
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Florin Irion <irionr@gmail.com> — 2026-03-03T17:35:18Z
Tom, Álvaro, thanks for the direction — I think the v9 patch addresses both of your concerns nicely. No extra test run. The event trigger is installed once in test_setup.sql, so it piggybacks on the existing regression suite. Every CREATE that fires ddl_command_end is checked inline — there is no separate execution of the core tests. Developers don't have to remember anything. The trigger inspects command_tag to derive the object type, then probes pg_catalog for a matching pg_get_<type>_ddl() function. If one exists it round-trips the object right there; if not it silently moves on. Adding a new pg_get_type_ddl() or pg_get_sequence_ddl() in the future automatically extends coverage to every CREATE of that type across the entire suite — zero changes to the trigger or to existing tests. Inline verification at creation time. The trigger does: get DDL → DROP → CREATE from DDL → get DDL → ASSERT match Because it runs at creation time, nothing yet depends on the new object, so the drop/recreate is safe. A session-local GUC guards against recursion (the recreate fires the trigger again). Bugs of omission. Tom raised the concern that comparing DDL output against DDL output could mask a missing property. The key thing here is that the test suite continues running with the recreated object. If the DDL function omits, say, a CHECK constraint, the recreated domain silently loses it — and any subsequent test that exercises that constraint will fail. So omissions surface as unexpected failures elsewhere in the suite, not just in the DDL comparison itself. With the current patch, 160 domains across 33 test files are automatically round-tripped. The dedicated object_ddl.sql file is gone — a small set of pg_get_domain_ddl() output-format tests (pretty printing, quoted identifiers, NOT VALID rendering, built-in type name shadowing, error cases) now lives in domain.sql alongside the rest of the domain coverage. v9 attached. While working on this I bumped into an unrelated crash and started a new thread [1] for it. [1] https://www.postgresql.org/message-id/c6fff161-9aee-4290-9ada-71e21e4d84de%40gmail.com -- Cheers, Florin EDB -- www.enterprisedb.com
-
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Florin Irion <irionr@gmail.com> — 2026-03-10T10:31:50Z
Hello, v10 rebased on latest master attached. Cheers, Florin www.enterprisedb.com
-
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Japin Li <japinli@hotmail.com> — 2026-03-12T01:20:09Z
Hi, Florin On Tue, 10 Mar 2026 at 16:01, Florin Irion <irionr@gmail.com> wrote: > Hello, > > v10 rebased on latest master attached. > Thanks for updating the patch. When I reviewed [1], I noticed that pg_get_database_ddl() uses spaces for indentation instead of tabs. What do you think? 1. + /* Add valid constraints */ + foreach(lc, validConstraints) + { + Oid constraintOid = lfirst_oid(lc); IMO, I think we can replace foreach() with foreach_oid(). 2. + foreach(lc, invalidConstraints) + { + Oid constraintOid = lfirst_oid(lc); Same as above. [1] https://www.postgresql.org/message-id/CANxoLDc6FHBYJvcgOnZyS%2BjF0NUo3Lq_83-rttBuJgs9id_UDg%40mail.gmail.com > Cheers, > Florin > > > www.enterprisedb.com > > [2. text/plain; v10-0001-Add-pg_get_domain_ddl-function-to-reconstruct-CR.patch]... -- Regards, Japin Li ChengDu WenWu Information Technology Co., Ltd. -
Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Florin Irion <irionr@gmail.com> — 2026-07-03T16:26:03Z
Hello, Thank you Japin Li for the last suggestions. I addressed them and as some sibling functions (pg_get_role_ddl(), pg_get_tablespace_ddl(), pg_get_database_ddl()) were merged in master since, I rebased and adjusted my patch to use the framework that is now present. Attaching V11 Cheers, Florin www.enterprisedb.com