Thread: fix for PL/PgSQL segfault

This patch fixes a segfault in the support for set-returning functions
PL/PgSQL that occurs when an undefined RECORD variable is returned with
RETURN NEXT. The fix is the one suggested by Tom on -hackers: a row of
NULL values is returned.

I've added a regression test for this behavior.

After the appropriate review, IMHO this patch should be applied to both
HEAD and REL_7_3_STABLE branches.

Cheers,

Neil
--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC

Neil Conway <neilc@samurai.com> writes:
> This patch fixes a segfault in the support for set-returning functions
> PL/PgSQL that occurs when an undefined RECORD variable is returned with
> RETURN NEXT. The fix is the one suggested by Tom on -hackers: a row of
> NULL values is returned.

Actually, the fix I had in mind was to cause the SELECT to assign a row
of nulls to the RECORD variable --- which is consistent with what it
will do if the SELECT target is a non-record variable.  That way, the
behavior is consistent for any subsequent use of the record variable,
not only RETURN NEXT.

Then, if rec->tup is found to be NULL in RETURN NEXT, that means no
attempt has ever been made to assign to the variable.  I'm undecided
about whether that case should return nulls as per your patch, or should
raise an error.  In the other places that access record variables, the
code seems to prefer to raise an error for a never-assigned-to record,
rather than silently substitute null.  I think we should probably try to
be consistent --- but does that mean raising an error here, or changing
those other places?  Any opinions?

            regards, tom lane

On Thu, 2003-01-16 at 12:18, Tom Lane wrote:
> Actually, the fix I had in mind was to cause the SELECT to assign a row
> of nulls to the RECORD variable

Heh, I just can't seem to get this patch right :-)

> Then, if rec->tup is found to be NULL in RETURN NEXT, that means no
> attempt has ever been made to assign to the variable.  I'm undecided
> about whether that case should return nulls as per your patch, or should
> raise an error.

It seems a little inconsistent to treat a "never-assigned-to" variable
differently than one which has been the target of a SELECT INTO that
returns zero rows, doesn't it?

In any case, I don't particularly mind which behavior we choose: when
there's a consensus, I'll send in a new version of the patch.

Cheers,

Neil
--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC

Neil Conway <neilc@samurai.com> writes:
>> Then, if rec->tup is found to be NULL in RETURN NEXT, that means no
>> attempt has ever been made to assign to the variable.  I'm undecided
>> about whether that case should return nulls as per your patch, or should
>> raise an error.

> It seems a little inconsistent to treat a "never-assigned-to" variable
> differently than one which has been the target of a SELECT INTO that
> returns zero rows, doesn't it?

Not entirely; the SELECT INTO is sufficient to determine the rowtype of
the variable, even if it can't stuff any data into the columns.  Thus,
for example, we can allow assignment to a field of the record variable
after such a select, whereas we really can't support it when the record
variable is completely without-form-and-void.  In the case of RETURN
NEXT, we are able to check that the record's rowtype matches what was
expected.

            regards, tom lane