Re: BUG #19848: BEGIN ATOMIC with unnamed/OUT params breaks restore and pg_upgrade

Hi Tom,

Sorry for the late reply, and thanks for the fix and for checking the
other callers of get_func_arg_info.

I tested your patch on current master. All the cases from my report now
create, dump and restore with identical results, and the core regression
tests (243) and plsample pass. pg_upgrade from a patched build to a patched
build works. Upgrading from 18.6 still fails with the empty identifier,
since the old server does the deparsing, so this would also need
backpatching to 18 to help pg_upgrade from there.

Agreed that the q2 case is a scoping problem and not part of this bug.
I don't have a better idea than printing $n, since any qualified name
can be captured by a FROM item with the same name.

Regards,
Atish

On Thu, 12 Nov 2026 at 08:43, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Atisss hh <125atishprak(at)gmail(dot)com> writes:
> > On Sat, 7 Nov 2026 at 20:46, PG Bug reporting form <
> noreply(at)postgresql(dot)org>
> > wrote:
> >> SQL-standard function bodies (BEGIN ATOMIC) handle parameter names
> >> wrongly when a function has OUT parameters before an input
> >> parameter, or an unnamed input alongside named parameters.
>
> Good catch --- obviously we've not tested the case with OUT parameters
> written first very much :-(
>
> >> A possible patch is attached
> (0001-fix-sql-function-parameter-names.patch).
>
> I don't like the details of this patch, though.
> interpret_function_parameter_list can be fixed more simply by moving
> the erroneous code, and the real problem with print_function_sqlbody
> is that it's using the wrong support function, and all of this is
> a bit underdocumented.
>
> I also looked around at other callers of get_func_arg_info, and
> noted that PL/Python is a little bit broken and plsample.c is a
> whole lot broken --- fortunately that's just test code.
>
> >> Related, and not fixed by the patch: a parameter deparsed as
> >> function.name can be captured by a table of the same name, which
> >> changes results after a restore:
> >>
> >> CREATE TABLE q2(a int);
> >> INSERT INTO q2 VALUES (111);
> >> CREATE FUNCTION q2(a int) RETURNS int LANGUAGE sql
> >> BEGIN ATOMIC SELECT $1 FROM q2; END;
> >> SELECT q2(5); -- 5 before dump and restore, 111 after
>
> Hmm. That's not related really, it's a scoping problem.
> The function deparses to
>
> CREATE OR REPLACE FUNCTION public.q2(a integer)
> RETURNS integer
> LANGUAGE sql
> BEGIN ATOMIC
> SELECT q2.a
> FROM q2;
> END;
>
> I think that interpreting q2.a as the table's column is fully correct
> here: the query's FROM clause is the most closely nested referent.
> So the real problem is that we're emitting the function parameter
> reference in a form that is fundamentally ambiguous, and that we do
> not have a way to make non-ambiguous. The only simple way out is to
> always print new-style-SQL-function-body function parameter references
> as $n not as names, which feels pretty sad. On the bright side, we
> could rip out a few dozen lines of buggy-anyway code in ruleutils.c.
> Anyone have a better idea?
>
> regards, tom lane
>
>