To: Joe Conway <mail@joeconway.com>
cc: "Hackers (PostgreSQL)" <pgsql-hackers@postgresql.org>
Subject: Re: Values list-of-targetlists patch for comments (was Re: [PATCHES]
	8.2 features?)
In-reply-to: <44CC0A66.6070903@joeconway.com> 
References: <44B69778.5050104@dunslane.net>
	<50461.84.63.113.55.1152832991.squirrel@pezone.net>
	<ECCAAC0A3182895D3D12B0E8@[172.26.14.247]>
	<44B7ADFC.90205@dunslane.net> <44BC0432.3040509@joeconway.com>
	<44BC661F.80609@joeconway.com> <44BCD06C.3060409@calorieking.com>
	<44BCF1D4.3010807@dunslane.net> <5842.1153246741@sss.pgh.pa.us>
	<44BD8854.9060506@calorieking.com> <15212.1153279954@sss.pgh.pa.us>
	<44BDB2F5.7080003@joeconway.com> <44BEA710.6060700@joeconway.com>
	<23198.1153367887@sss.pgh.pa.us> <44BF0382.2010501@joeconway.com>
	<23454.1153370214@sss.pgh.pa.us> <44BF122F.1080200@joeconway.com>
	<23927.1153373971@sss.pgh.pa.us> <44C00E1E.1000405@joeconway.com>
	<14047.1153442773@sss.pgh.pa.us> <44C4451A.4010906@joeconway.com>
	<11495.1153757531@sss.pgh.pa.us> <44C4F625.2050304@joeconway.com>
	<12029.1153760969@sss.pgh.pa.us> <44CC0A66.6070903@joeconway.com>
Comments: In-reply-to Joe Conway <mail@joeconway.com>
	message dated "Sat, 29 Jul 2006 18:24:54 -0700"
Date: Tue, 01 Aug 2006 16:15:51 -0400
Message-ID: <16429.1154463351@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>

I've found a problem with the VALUES-as-RTE approach:

regression=# create table src(f1 int, f2 int);
CREATE TABLE
regression=# create table log(f1 int, f2 int, tag text);
CREATE TABLE
regression=# insert into src values(1,2);
INSERT 0 1
regression=# create rule r2 as on update to src do
regression-# insert into log values(old.*, 'old'), (new.*, 'new');
CREATE RULE
regression=# update src set f2 = f2 + 1;
server closed the connection unexpectedly

The problem with this is that the rewriter is substituting Vars
referencing "src" into the values lists of the VALUES RTE, within
a query that looks like a Cartesian product of src and *VALUES*:

regression=# explain update src set f2 = f2 + 1;
                             QUERY PLAN
--------------------------------------------------------------------
 Nested Loop  (cost=0.00..97.62 rows=3880 width=40)
   ->  Values Scan on "*VALUES*"  (cost=0.00..0.02 rows=2 width=40)
   ->  Seq Scan on src  (cost=0.00..29.40 rows=1940 width=0)

 Seq Scan on src  (cost=0.00..34.25 rows=1940 width=14)
(5 rows)

The ValuesScan node doesn't have access to the values of the current
row of src ... indeed, the planner doesn't know that it shouldn't
put the VALUES on the outside of the join, as it's done here, so
there *isn't* a current row of src.

AFAICT, the only way to make this work would be to implement SQL99's
LATERAL construct (or something pretty close to it --- I'm not entirely
sure I understand what LATERAL is supposed to do) so that the rewritten
query could be expressed like

	insert into log select ... from src, LATERAL VALUES(src.f1, ...)

That's obviously not something we can get done for 8.2.

We could maybe kluge something to work for 8.2 if we were willing to
abandon the VALUES-as-RTE approach and go back to the notion of some
kind of multiple targetlist in a Query.  I'm disinclined to do that
though, because as I've been working with your patch I've come to agree
that the RTE solution is a pretty clean one.

What I'm inclined to do for 8.2 is to disallow OLD/NEW references in
multi-element VALUES clauses; the feature is still tremendously useful
without that.

			regards, tom lane