Re: json_populate_record issue - TupleDesc reference leak

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: json_populate_record issue - TupleDesc reference leak
Date: 2015-02-26 22:31:44
Message-ID: 30106.1424989904@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> This doesn't look quite right. Shouldn't we unconditionally release the
> Tupledesc before the returns at lines 2118 and 2127, just as we do at
> the bottom of the function at line 2285?

I think Pavel's patch is probably OK as-is, because the tupdesc returned
by get_call_result_type isn't reference-counted; but I agree the code
would look cleaner your way. If the main exit isn't bothering to
distinguish this then the early exits should not either.

What I'm wondering about, though, is this bit at line 2125:

/* same logic as for json */
if (!have_record_arg && rec)
PG_RETURN_POINTER(rec);

If that's supposed to be the same logic as in the other path, then how
is it that !have_record_arg has anything to do with whether the JSON
object is empty? Either the code is broken, or the comment is.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2015-02-26 22:34:46 Re: logical column ordering
Previous Message Andres Freund 2015-02-26 22:31:16 Re: Renaming MemoryContextResetAndDeleteChildren to MemoryContextReset