[DEPARSE] Parameters in SQL-standard function bodies do not read back

Hi hackers,

ruleutils.c prints a parameter of an SQL-standard function body
(BEGIN ATOMIC or RETURN) by its name. When a relation has the same
name as the function, the parameter is read back as a column of that
relation; without relations, a parameter used as a sort key is read
back as the output column of the same name; and an unnamed parameter
is printed as an empty string. A function created again by dump and
restore, or from pg_get_functiondef() output, then silently gives a
different result, or is not created at all. The two attached patches
add tests and fix it.

The problem
-----------

get_parameter() prints a parameter as funcname.argname when the query
has relations, as the bare argname otherwise, and as $N when it has no
name. In three cases the output is not read back as the same
parameter.

1. With a relation named like the function, the parameter is read
back as a column of that relation.

The parser resolves funcname.argname as a column reference first, and
sql_fn_post_column_ref() never overrides that. A column added after
the function was created does the same:

create table f (x int);
insert into f values (5);
create function f(a int) returns int
begin atomic select a + x from f; end;
alter table f add column a int default 100;
select f(10); -- 15

before: SELECT (f.a + f.x) FROM f;
created again, f(10) is 105
after: SELECT ($1 + f.x) FROM f;

It need not be a user column. It is also read back as a system
column, or as a call of a function taking the whole row
(argname(funcname.*)). With a pg_catalog function name no DDL is
needed:

create table f (x int);
insert into f values (1);
create function f(num_nulls int) returns int
begin atomic select num_nulls + x from f; end;
select f(7); -- 8

before: SELECT (f.num_nulls + f.x) FROM f;
read as num_nulls(f.*), f(7) is 1
after: SELECT ($1 + f.x) FROM f;

With a parameter named cmin, the body reads back as the cid column and
creating it again fails with "return type mismatch". It is not only
table names that clash with the function name: an alias of a FROM
item, a JOIN ... USING (..) AS alias and the RETURNING aliases for OLD
and NEW (including the default old and new) do the same.

2. Without relations, a parameter used as an ORDER BY or DISTINCT ON
key is read back as the output column of the same name.

The bare name is printed, and findTargetlistEntrySQL92() matches it
against the output column names first:

create function f(a int) returns int[]
return array(select 4 - generate_series(1, 3) as a
order by $1);
select f(5); -- {3,2,1}

before: ... AS a ORDER BY a)
created again, f(5) is {1,2,3}
after: ... AS a ORDER BY $1)

If the output column type has no ordering operator (point, say),
creating it again fails with "could not identify an ordering
operator". On 15-17 a GROUP BY key is affected the same way. From 18
on, a query with GROUP BY has an RTE_GROUP entry, so the name is
qualified and this does not apply.

3. An unnamed parameter is printed as an empty string.

Unnamed parameters are stored as empty strings, not NULLs, so they are
printed as an empty identifier instead of $N (SELECT (a + "")), which
does not read back.

pg_dump prints with the same code. In a real dump and restore, the
num_nulls example above changed its result from 8 to 1, and the point
example was not restored at all.

The fix
-------

0002 prints the parameter as $N in all three cases.

1. When a name visible as a relation qualifier at any query level
equals the function name. The names looked at are range table
entry names, JOIN/USING aliases and the RETURNING aliases for OLD
and NEW. It looks only at the names, not at the columns, since
the output must stay correct after columns are added. When no
such name is visible, the qualifier funcname cannot be bound to
any relation, so funcname.argname is safe.
2. When, without relations, the parameter is an ORDER BY, DISTINCT
ON or GROUP BY key.
3. When its name is empty, which is treated as no name.

Otherwise the output is unchanged: the bare name in cases like
"RETURN a + b", and funcname.argname in other queries.

The check is conservative. It does not work out whether the name is
actually in scope at the point of reference, so it may print $N where
it is not strictly needed. Among the existing outputs, only \sf
ts_debug(text) in the psql test changes. The function in its FROM
clause carries the default alias ts_debug, so ts_debug.document is
now shown as $1. That reference was not actually ambiguous, since a
function's arguments cannot see the alias of its own range table
entry, but the check does not distinguish that case. $1 is also what
the function's source uses.

Tests
-----

0001 adds the tests with the output before the fix, and 0002 updates
it, so that the diff shows what changes. Each case creates a
function, calls it, creates it again from its pg_get_functiondef()
output and calls it again, so the expected output shows whether the
results agree. The cases cover the problems above one by one:
columns added later or renamed, a whole-row function, a system
column, a join alias, a JOIN/USING alias, the OLD and NEW aliases
(explicit and default), ORDER BY, DISTINCT ON, GROUP BY, a sort key
that fails to parse again, and unnamed parameters.

Back branches
-------------

I think this should be back-patched to 15. The two patches apply as
they are to REL_18_STABLE and REL_19_STABLE. The versions for 15-17
are attached separately as .txt files. They differ as follows:

- 17: there is no RETURNING OLD/NEW, so that check and its tests
are dropped.
- 16: as 17, and foreach is used instead of foreach_ptr.
- 15: as 16, and deparse_context has no varInOrderBy field, so it
is added.

On 15-17 the GROUP BY test shows the bug before the fix (the result
changes after the function is created again). On every branch from
15 to master, make check passes with 0001 alone, and check-world
passes with both patches.

14 has all the same problems. The 15 fix applies to 14 as it is,
and the test patch only needs its block moved, since the file is
named create_function_3.sql there. I did not attach a version for
14, since 14 is about to reach end of life, but I can send one if
wanted.

This came out of an AI-assisted audit of ruleutils.c. I will post the
other problems it found as separate reports as I work through them.

Best regards,
Henson