From: Tom Lane <tgl@sss.pgh.pa.us>
To: Yahor Yuzefovich <yahor@cockroachlabs.com>
cc: pgsql-bugs@lists.postgresql.org,
        Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Subject: Re: A server crash with a SQL procedure returning a user-defined type
 on 14.8
In-reply-to: 
 <CABz5gWHSjj2df6uG0NRiDhZ_Uz=Y8t0FJP-_SVSsRsnrQT76Gg@mail.gmail.com>
References: 
 <CABz5gWHSjj2df6uG0NRiDhZ_Uz=Y8t0FJP-_SVSsRsnrQT76Gg@mail.gmail.com>
Comments: In-reply-to Yahor Yuzefovich <yahor@cockroachlabs.com>
	message dated "Mon, 11 Mar 2024 15:19:37 -0700"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1718612.1710273043.1@sss.pgh.pa.us>
Date: Tue, 12 Mar 2024 15:50:43 -0400
Message-ID: <1718613.1710273043@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1718613.1710273043%40sss.pgh.pa.us>
Precedence: bulk

Yahor Yuzefovich <yahor@cockroachlabs.com> writes:
> CREATE TYPE typ AS (a INT, b INT); CREATE PROCEDURE p_udt(OUT typ) AS $$
> SELECT (1, 2); $$ LANGUAGE SQL; CALL p_udt(NULL);

Thanks for the report.  What seems to be happening is that functions.c
is getting confused as to whether it should return a record containing
a record, or just a record.  check_sql_fn_retval explains:

         * If the target list has one non-junk entry, and that expression has
         * or can be coerced to the declared return type, take it as the
         * result.  This allows, for example, 'SELECT func2()', where func2
         * has the same composite return type as the function that's calling
         * it.  This provision creates some ambiguity --- maybe the expression
         * was meant to be the lone field of the composite result --- but it
         * works well enough as long as we don't get too enthusiastic about
         * inventing coercions from scalar to composite types.

As far as I know, that is fine for functions.  But it's not fine for
procedures: those are marked as returning RECORD if there are any
output parameters at all, and the code for CALL expects that it's
going to get back a record containing one column per output parameter,
so we can't flatten that into a record containing two ints.
This has been busted since we invented procedures, I think.

This is easy to fix if we add a parameter to check_sql_fn_retval
indicating whether we're considering a function or a procedure.
While that's not problematic in HEAD, I'm worried that there might
be external callers of that function in the back branches.  I guess
we can use the old trick of making the existing function into a
wrapper in the back branches.

			regards, tom lane