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

Поиск
Список
Период
Сортировка

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

От
Tim Waizenegger
Дата:
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
Дата:
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
Дата:
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
Дата:
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
Дата:
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
Дата:
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
Дата:


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
Дата:
>> 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)

Вложения

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

От
Florin Irion
Дата:
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

От
Man Zeng
Дата:
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

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

От
Chao Li
Дата:

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







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

От
Neil Chen
Дата:
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
Дата:
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)



Вложения

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

От
Haritabh Gupta
Дата:
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

Вложения