On Sun, Jul 19, 2009 at 12:39 PM, Tom Lane<t...@sss.pgh.pa.us> wrote: > Robert Haas <robertmh...@gmail.com> writes: >> I think this is a great feature, and it would be REALLY great if it >> supported UPDATE and DELETE as well. > > It won't get applied until it does, and I imagine the patch author > wasn't expecting any differently. The submission was clearly marked > "WIP" not "ready to apply". > >> However, it sounds to me like this is going to need more reworking >> than is going to get done in the next week or two. > > Yeah. I did a quick scan of the patch and was distressed at how much of > it seemed to be addition of new code; that implies that he's more or > less duplicated the top-level executor processing. > > The way that I think this should be approached is > (1) a code-refactoring patch that moves INSERT/UPDATE/DELETE control > into plan nodes; then > (2) a feature patch that makes use of that to expose RETURNING in CTEs. > > One thing that's not totally clear to me is whether we'd like to use > control plan nodes all the time, or only for statements with RETURNING. > The nice thing about the latter approach is that there's a well-defined > meaning for the plan node's targetlist. (Its input is the stuff to be > stored, its output is the RETURNING result; much cleaner than the way > RETURNING is bolted onto the executor now.) If we do it all the time > then the control nodes would have dummy targetlists for non-RETURNING > cases, which is a little bit ugly. OTOH it may not be practical to > handle it like that without a lot of code duplication. > > Another thing to think about is whether there should be three types > of control nodes or only one. > > But anyway, my $0.02 is that the *first* order of business is to propose > a suitable refactoring of execMain.c. We skipped doing that when we > first added RETURNING, but it's time to make it happen. Not fasten > another kluge on top of a kluge.
Tom, This is a great review and great input. I wish there were enough hours in the day for you to do something like this for every patch. I feel good about saying that this is Returned With Feedback at this point, and I see that Jaime Casanova has already made that update. Thanks very much, ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
