Fix variadic argument types for pg_get_xxx_ddl() functions
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]/messages/by-id/CANxoLDe_GOXyyZs7GN3VUJoT+o1DwqZGhid-TWWvS0D8d5ggYw@mail.gmail.com 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]: /messages/by-id/CANxoLDe_GOXyyZs7GN3VUJoT+o1DwqZGhid-TWWvS0D8d5ggYw@mail.gmail.com
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
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] /messages/by-id/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/
Attachments:
v1-0001-Tighten-opr_sanity-check-for-variadic-functions.patchapplication/octet-stream; name=v1-0001-Tighten-opr_sanity-check-for-variadic-functions.patch; x-unix-mode=0644Download+9-10
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.evan.chao@gmail.com> wrote:
Show quoted text
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 => 'regroletext',
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]
/messages/by-id/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/
On Jun 23, 2026, at 16:34, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
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)'); +
Hi Akshay,
I actually considered adding VARIADIC ARRAY test cases when I worked on the patch, but I intentionally avoided them because I found that similar variadic functions, such as json_extract_path and jsonb_extract_path, don’t have such tests either. Also, the sanity check is now able to detect this problem.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Chao Li <li.evan.chao@gmail.com> writes:
On Jun 23, 2026, at 10:11, Chao Li <li.evan.chao@gmail.com> wrote:
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:
Ouch. Yes, that regression test is clearly one brick shy of a load.
We need to update the documentation for these functions too, and maybe
some other places.
regards, tom lane
I wrote:
Ouch. Yes, that regression test is clearly one brick shy of a load.
We need to update the documentation for these functions too, and maybe
some other places.
After looking around, I couldn't find anything but the documentation
that needed to be fixed, so done.
regards, tom lane
On Tue, Jun 23, 2026 at 3:07 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
I wrote:
Ouch. Yes, that regression test is clearly one brick shy of a load.
We need to update the documentation for these functions too, and maybe
some other places.After looking around, I couldn't find anything but the documentation
that needed to be fixed, so done.
Thanks for fixing.
cheers
andrew