> On Jun 23, 2026, at 10:11, Chao Li <[email protected]> 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-twwvs0d8d5g...@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/
v1-0001-Tighten-opr_sanity-check-for-variadic-functions.patch
Description: Binary data
