Re: Fix variadic argument types for pg_get_xxx_ddl() functions - Mailing list pgsql-hackers

From Akshay Joshi
Subject Re: Fix variadic argument types for pg_get_xxx_ddl() functions
Date
Msg-id CANxoLDfYD9_Mv7beHrLOWrWh_d9_dODWDxtityx3vZ05G5nZ9g@mail.gmail.com
Whole thread
In response to Re: Fix variadic argument types for pg_get_xxx_ddl() functions  (Chao Li <li.evan.chao@gmail.com>)
List pgsql-hackers
Hi Chao,

Patch looks good to me. I have added the following test cases in my local version in "src/test/modules/test_misc/t/012_ddlutils.pl". If possible add those and resend the patch

+# Options passed via VARIADIC ARRAY
+$result = $node->safe_psql('postgres',
+       q{SELECT * FROM pg_get_role_ddl('regress_role_ddl_test2',
+           VARIADIC ARRAY['pretty', 'false'])});
+unlike($result, qr/\n\s+SUPERUSER/,
+       'role VARIADIC ARRAY pretty=false suppresses indentation');
+like($result, qr/SUPERUSER/, 'role VARIADIC ARRAY pretty=false still emits attributes');


+# Options passed via VARIADIC ARRAY
+$result = ddl_filter(
+       $node->safe_psql(
+               'postgres',
+               q{SELECT pg_get_database_ddl
+         FROM pg_get_database_ddl('regression_ddlutils_test',
+           VARIADIC ARRAY['pretty', 'false'])}));
+unlike($result, qr/\n\s+WITH TEMPLATE/,
+       'database VARIADIC ARRAY pretty=false suppresses indentation');
+like(
+       $result,
+       qr/CREATE DATABASE regression_ddlutils_test/,
+       'database VARIADIC ARRAY still emits CREATE');


+# Options passed via VARIADIC ARRAY (name variant)
+$result = $node->safe_psql(
+       'postgres',
+       q{SELECT * FROM pg_get_tablespace_ddl('regress_allopt_tblsp',
+         VARIADIC ARRAY['pretty', 'false'])});
+unlike($result, qr/\n\s+OWNER/,
+       'tablespace VARIADIC ARRAY pretty=false suppresses indentation (name)');
+like(
+       $result,
+       qr/OWNER regress_role_ddl_test1/,
+       'tablespace VARIADIC ARRAY still emits OWNER (name)');
+
+# Options passed via VARIADIC ARRAY (oid variant)
+$result = $node->safe_psql(
+       'postgres', q{
+       SELECT * FROM pg_get_tablespace_ddl(
+         (SELECT oid FROM pg_tablespace
+          WHERE spcname = 'regress_allopt_tblsp'),
+         VARIADIC ARRAY['pretty', 'false'])});
+unlike($result, qr/\n\s+OWNER/,
+       'tablespace VARIADIC ARRAY pretty=false suppresses indentation (oid)');
+like(
+       $result,
+       qr/CREATE TABLESPACE regress_allopt_tblsp/,
+       'tablespace VARIADIC ARRAY still emits CREATE (oid)');
+

On Tue, Jun 23, 2026 at 1:50 PM Chao Li <li.evan.chao@gmail.com> wrote:


> On Jun 23, 2026, at 10:11, Chao Li <li.evan.chao@gmail.com> wrote:
>
> Hi,
>
> While testing "[76e514ebb] Add pg_get_role_ddl() function" as well as the other pg_get_xxx_ddl() functions, I noticed that their pg_proc.dat definitions use "text" rather than "_text" for variadic argument types. For example:
> ```
> { oid => '6501', descr => 'get DDL to recreate a role',
> proname => 'pg_get_role_ddl', prorows => '10', provariadic => 'text',
> proisstrict => 'f', proretset => 't', provolatile => 's',
> pronargdefaults => '1', prorettype => 'text', proargtypes => 'regrole text',
> proallargtypes => '{regrole,text}', proargmodes => '{i,v}',
> proargdefaults => '{NULL}', prosrc => 'pg_get_role_ddl' },
> ```
>
> I spotted this problem some time ago, but I wasn’t sure whether it was a real issue worth fixing. When I saw a new patch [1] for pg_get_table_ddl() that had the same problem, I asked about it there. Akshay confirmed the problem and pointed me to the opr_sanity check. Thank you very much, Akshay.
>
> Then I checked why the sanity check didn’t report the issue. I found that the check used != for the comparison. When the final argument type is not an array type, the subquery returns NULL, and NULL != provariadic evaluates to NULL rather than true. I fixed the check to use IS DISTINCT FROM. After that, it reported the mismatch:
> ```
> % make -C src/test/regress check-tests TESTS='test_setup opr_sanity'
> # +++ regress check in src/test/regress +++
> # initializing database system by copying initdb template
> # using temp instance on port 58928 with PID 36865
> ok 1         - test_setup                                243 ms
> # diff -U3 /Users/chaol/Documents/code/postgresql/src/test/regress/expected/opr_sanity.out /Users/chaol/Documents/code/postgresql/src/test/regress/results/opr_sanity.out
> # --- /Users/chaol/Documents/code/postgresql/src/test/regress/expected/opr_sanity.out   2026-06-23 07:58:52
> # +++ /Users/chaol/Documents/code/postgresql/src/test/regress/results/opr_sanity.out    2026-06-23 07:59:41
> # @@ -488,9 +488,13 @@
> #                 FROM pg_type t
> #                 WHERE t.typarray = proargtypes[array_length(proargtypes, 1)-1])
> #       END IS DISTINCT FROM provariadic;
> # - oid | provariadic | proargtypes
> # ------+-------------+-------------
> # -(0 rows)
> # +                  oid                  | provariadic |       proargtypes
> # +---------------------------------------+-------------+--------------------------
> # + pg_get_role_ddl(regrole,text)         | text        | [0:1]={regrole,text}
> # + pg_get_tablespace_ddl(oid,text)       | text        | [0:1]={oid,text}
> # + pg_get_tablespace_ddl(name,text)      | text        | [0:1]={name,text}
> # + pg_get_database_ddl(regdatabase,text) | text        | [0:1]={regdatabase,text}
> # +(4 rows)
> #
> #  -- Check that all and only those functions with a variadic type have
> #  -- a variadic argument.
> not ok 2     - opr_sanity                               8938 ms
> 1..2
> # 1 of 2 tests failed.
> ```
>
> After updating pg_proc.dat to change those "text" entries to "_text", the test passed.
>
> See the attached patch for details.
>
> [1] https://postgr.es/m/CANxoLDe_GOXyyZs7GN3VUJoT+o1DwqZGhid-TWWvS0D8d5ggYw@mail.gmail.com
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
>
>
>

Oops! Missed the attachment.

--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/




pgsql-hackers by date:

Previous
From: Chao Li
Date:
Subject: Re: Fix variadic argument types for pg_get_xxx_ddl() functions
Next
From: Dean Rasheed
Date:
Subject: Re: Add PRODUCT() aggregate function