Re: Fix variadic argument types for pg_get_xxx_ddl() functions

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net>
Subject: Re: Fix variadic argument types for pg_get_xxx_ddl() functions
Date: 2026-06-23 08:34:42
Message-ID: CANxoLDfYD9_Mv7beHrLOWrWh_d9_dODWDxtityx3vZ05G5nZ9g@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: 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
<http://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(dot)evan(dot)chao(at)gmail(dot)com> wrote:

>
>
> > On Jun 23, 2026, at 10:11, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)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/
>
>
>
>
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dean Rasheed 2026-06-23 08:37:11 Re: Add PRODUCT() aggregate function
Previous Message Chao Li 2026-06-23 08:17:37 Re: Fix variadic argument types for pg_get_xxx_ddl() functions