Re: Error in PQsetvalue

On Wed, Jun 8, 2011 at 11:03 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Merlin Moncure <mmoncure(at)gmail(dot)com> writes:
>> On Wed, Jun 8, 2011 at 10:18 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Merlin Moncure <mmoncure(at)gmail(dot)com> writes:
>>>> I went ahead and tested andrew's second patch -- can we get this
>>>> reviewed and committed?
>
>>> Add it to the upcoming commitfest.
>
>> It's a client crashing bug in PQsetvalue that goes back to 9.0 :(.
>
> I was under the impression that this was extending PQsetvalue to let it
> be used in previously unsupported ways, ie, to modify a server-returned
> PGresult.  That's a feature addition, not a bug fix.

It's neither -- it's documented libpq behavior: "The function will
automatically grow the result's internal tuples array as needed.
However, the tup_num argument must be less than or equal to PQntuples,
meaning this function can only grow the tuples array one tuple at a
time. But any field of any existing tuple can be modified in any
order. "

Andrew was briefly flirting with a proposal to tweak this behavior,
but withdrew the idea.

> it's a feature addition I approve of.  I think serious consideration
> ought to be given to locking down returned results so PQsetvalue refuses
> to touch them, instead.  Otherwise we're likely to find ourselves unable
> to make future optimizations because we have to support this
> barely-used-by-anybody corner case.

I think that's debatable, but I'm not going to argue this yea or nea.
But I will say that maybe we shouldn't confuse behavior issues with
bug fix either way...patch the bug, and we can work up a patch to lock
down the behavior and the docs if you want it that way, but maybe we
could bikeshed a bit on that point.

merlin