Fix variadic argument types for pg_get_xxx_ddl() functions

Started by Chao Liabout 22 hours ago7 messageshackers
Jump to latest
#1Chao Li
li.evan.chao@gmail.com

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/

#2Chao Li
li.evan.chao@gmail.com
In reply to: Chao Li (#1)
Re: Fix variadic argument types for pg_get_xxx_ddl() functions

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
#3Akshay Joshi
akshay.joshi@enterprisedb.com
In reply to: Chao Li (#2)
Re: Fix variadic argument types for pg_get_xxx_ddl() functions

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&gt;*&quot;. 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 => '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/

#4Chao Li
li.evan.chao@gmail.com
In reply to: Akshay Joshi (#3)
Re: Fix variadic argument types for pg_get_xxx_ddl() functions

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/

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Chao Li (#2)
Re: Fix variadic argument types for pg_get_xxx_ddl() functions

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

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#5)
Re: Fix variadic argument types for pg_get_xxx_ddl() functions

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

#7Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#6)
Re: Fix variadic argument types for pg_get_xxx_ddl() functions

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