Re: Writeable CTE patch

Tom Lane wrote:
> 1. I thought we'd agreed at
> http://archives.postgresql.org/pgsql-hackers/2009-10/msg00558.php
> that the patch should support WITH on DML statements, eg
> with (some-query) insert into foo ...
> This might not take much more than grammar additions, but it's
> definitely lacking at the moment.

Ok, I added these.

> One thing that really does have to draw an error is that AFAIR the current
> rule feature doesn't enforce that a rewritten query produce the same type
> of output that the original would have. We just ship off whatever the
> results are to the client, and let it sort everything out. In a DML WITH
> query, though, I think we do have to insist that the rewritten query(s)
> still produce the same RETURNING rowtype as before.

Agreed.

> 3. I'm pretty unimpressed with the code added to ExecutePlan. It knows
> way more than it ought to about CTEs, and yet I don't think it's doing the
> right things anyway --- in particular, won't it run the "leader" CTE more
> than once if one CTE references another?

Yes. Are you suggesting something more intelligent to avoid scanning
the CTE more than once or..?

> I think it would be better if
> the PlannedStmt representation just told ExecutePlan what to do, rather
> than having all these heuristics inside ExecutePlan.

Yup, seems like a better choice.

> (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?

> 4. As previously noted, the changes to avoid using es_result_relation_info
> are broken and need to be dropped from the patch.

Done. I kept the logic for result relations to allow nested ModifyTable
nodes, but I don't think it ever did the right thing with EvalPlanQual()
and nested nodes. I'll have think about that.

Regards,
Marko Tiikkaja