On Sun, 2005-05-15 at 15:09 -0400, Tom Lane wrote: > I'm planning to change ExecRestrPos and the routines it calls so that > an updated TupleTableSlot holding the restored-to tuple is explicitly > returned. > > Currently, since nothing is explicitly done to the result Slot of a > plan node when we restore its position, you might think that the Slot > still points at the tuple that was current just before the Restore. > You'd be wrong though, at least for seqscan and indexscan plans > (I haven't looked yet at the other node types that support > mark/restore). The reason is that the restore operation changes > the contents of a HeapTupleData struct in the scan state (rs_ctup > or xs_ctup) and all that the Slot really contains is a pointer to > that struct. > > Now this is really bad. In the first place, the Slot thinks it > has a pin on the buffer containing its current tuple. After a > Restore, it may have pin on the wrong buffer.
Sounds terrible. Is this a particular bug you're fixing? > It seems to be > sheer chance that we've not had bugs due to this. It isn't a very common case, thats why. You'd need to have value duplicates in both joined columns, which is effectively a product join. Granted it is syntactically allowable SQL. AFAICS all joins should be between relations 1:1 or 1:M. A direct M:M join is actually missing out the associative relation, or a non-key self join. Such a join would rarely if ever have any correct and real meaning. I can think of a few, but mostly its just incorrectly coded SQL, or use of special values (e.g. blanks) rather than NULLs. So my guess is that ExecRestrPos is almost never called. > (The underlying > scan does have pin on the right buffer, but one can easily imagine > sequences in which the scan could be cleared while the Slot is > still assumed valid.) As of CVS tip the consequences could be > even worse, because the Slot may contain some pointers to extracted > fields of the tuple, and these pointers are now out of sync with > the tuple that the Slot really contains. > > So I think that it's essential that we explicitly update the scan > result Slot during ExecRestrPos. Yeh, no problem. Best Regards, Simon Riggs ---------------------------(end of broadcast)--------------------------- TIP 8: explain analyze is your friend