Re: Review of Writeable CTE Patch

On 2010-01-26 17:11, Alvaro Herrera wrote:
> Merlin Moncure escribió:
>
>> *) CopySnapshot was promoted from static. Is this legal/good idea?
>> Is a wrapper more appropriate?
>
> Hmm ... I wonder why isn't the patch doing RegisterSnapshot with the
> passed snapshot directly -- why is it necessary to create a new copy of
> it? (I notice that only one of the arms in that "if" creates a copy;
> if that is correct, I think it warrants a comment explaining why).

Per discussion here:
http://archives.postgresql.org/pgsql-hackers/2009-11/msg01964.php the
executor copies the snapshot if it plans on modifying it. A comment
explaining this might be in order.

> If the copy is necessary (e.g. because the snapshot must not be modified
> externally, and there's actual risk that it is), then maybe it would be
> better to create a new function RegisterSnapshotCopy instead?

Sounds reasonable.

Regards,
Marko Tiikkaja