Обсуждение: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement

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)

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

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)

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/

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)

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.

>> 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_ddlpatch. 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)

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)

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
apologiesfor the mix-up! Let’s just continue the discussion here as planned.
 

-- 
Regrads,
Man Zeng

> 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
notexist”. 

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/

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
apologiesfor 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’
doesnot 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)

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
hasdefined 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
wherethe `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`
becausethat is where the `CREATE DOMAIN...` tests reside. 

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
hasdefined 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
wherethe `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`
becausethat 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

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

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

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

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

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>

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

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

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