Should a plan node's result tuple slot be read-only to caller?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Should a plan node's result tuple slot be read-only to caller?
Date: 2005-11-14 15:14:53
Message-ID: 11219.1131981293@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I looked into Frank van Vugt's recent report of bizarre behavior in 8.1:
http://archives.postgresql.org/pgsql-bugs/2005-11/msg00121.php

The problem occurs because execMain.c's ExecInsert() replaces the
contents of the TupleTableSlot passed to it with whatever the trigger
hands back. This slot is the result slot of the top-level plan node,
which in the case at hand is a Unique node. In 8.0 and before, this does
not result in a failure, because Unique is keeping a separate copy of
its last output tuple to compare to; the 8.0 code comments:

/*
* We have a new tuple different from the previous saved tuple (if
* any). Save it and return it. We must copy it because the source
* subplan won't guarantee that this source tuple is still accessible
* after fetching the next source tuple.
*
* Note that we manage the copy ourselves. We can't rely on the result
* tuple slot to maintain the tuple reference because our caller may
* replace the slot contents with a different tuple. We assume that
* the caller will no longer be interested in the current tuple after
* he next calls us.
*
* tgl 3/2004: the above concern is no longer valid; junkfilters used to
* modify their input's return slot but don't anymore, and I don't
* think anyplace else does either. Not worth changing this code
* though.
*/

In connection with the "virtual tuple slot" optimization added for 8.1,
I rewrote this code and got rid of the supposedly-redundant extra tuple
copy. nodeUnique is now comparing the next input tuple directly to the
contents of its result slot, and so it gets fooled when the caller
changes that slot. (IOW, the comment I added in 3/2004 was wrong...)

The minimum-change way to fix the bug would be to revert the logic
change in nodeUnique.c and go back to maintaining a separate tuple copy.
But I'm thinking that this sort of thing could happen again. ISTM
it's not intuitive to allow a plan node's caller to scribble on the plan
node's result slot. An alternative solution would be to require
execMain.c to keep an extra tuple table slot that has no other purpose
than to temporarily hold replacement tuples during ExecInsert and
ExecUpdate.

I'm leaning towards the extra-slot approach, but wondered if anyone
has comments or better ideas.

regards, tom lane

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Gavin Sherry 2005-11-14 15:22:15 Re: outer joins and for update
Previous Message Jaime Casanova 2005-11-14 15:09:06 Re: functions marked STABLE not allowed to do INSERT