On 6/3/2011 7:14 PM, Merlin Moncure wrote:
On Fri, Jun 3, 2011 at 6:22 PM, Andrew Chernow<a...@esilo.com>  wrote:
Actually, the original idea, as I stated UT, was to allow adding tuples in
any order as well as overwriting them.  Obviously lost that while trying to
get libpqtypes working, which only appends.

Well, that may or not be the case, but that's irrelevant.  This has to
be backpatched to 9.0 and we can't use a bug to sneak in a behavior
change...regardless of what's done, it has to work as documented --
the current behavior isn't broken.  If we want PQsetvalue to support
creating tuples past the insertion point, that should be proposed for
9.2.


Well, not really irrelevant since understanding the rationale behind changes is important. I actually reversed my opinion on this and was preparing a patch that simply did a memset in pqAddTuple. See below for why....

You need to have insertion point zero'd in either case.  Look at the code.
  It only allocates when appending *AND* the tuple at that index is NULL.  If
pqAddTuple allocated the table, the tuple slots are uninitialized memory,
thus it is very unlikely that the next tuple slot is NULL.

I disagree -- I think the fix is a one-liner.  line 446:
if (tup_num == res->ntups&&  !res->tuples[tup_num])

should just become
if (tup_num == res->ntups)

also the memset of the tuple slots when the slot array is expanded can
be removed. (in addition, the array tuple array expansion should
really be abstracted, but that isn't strictly necessary here).


All true. This is a cleaner fix to something that was in fact broken ;) You want to apply it?

The fact that the tuples were being zero'd plus the NULL check is evidence of the original intent; which is what confused me. I found internal libpqtypes notes related to this change, it actually had to do with producing a result with dead tuples that would cause PQgetvalue and others to crash. Thus, it seemed better to only allow creating a result that is always *valid*.

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to