Re: Writeable CTE patch

[Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>]

Marko Tiikkaja <marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi> writes:
> Tom Lane wrote:
>> (BTW, I also think
>> it would work better if you had the CommandCounterIncrement at the bottom
>> of the loop, after the subquery execution not before it.  But I'm not sure
>> it's safe for ExecutePlan to be modifying the snapshot it's handed anyway.)

> Agreed.  I'm a bit lost here with the snapshot business; is doing this
> work in ExecutePlan() out of the question or is it just that what I'm
> doing is wrong?

I think it's not a good idea for ExecutePlan to be scribbling on the
executor's input, and the provided snapshot is definitely an input.
It might accidentally fail to fail in the present system, but it would
always be a hazard.  The only thing that I'd be comfortable with is
copying the snap and modifying the copy.  This might be okay from a
performance standpoint if it's done at the bottom of the loop (ie,
only when you actually have at least one writable CTE).  It would be
altogether cleaner though if the CommandCounterIncrement responsibility
were in the same place it is now, ie the caller of the executor.  Which
could be possible if we restructure the rewriter/planner output as a
list of Queries instead of just one.  I'm not currently sure how hard
that would be, though; it might not be a practical answer.

			regards, tom lane