Robert Haas <robertmhaas@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.
regards, tom lane
