(2014/09/12 16:03), Albe Laurenz wrote:
Tom Lane wrote:
Stephen Frost <sfr...@snowman.net> writes:
I have to admit that, while I applaud the effort made to have this
change only be to postgres_fdw, I'm not sure that having the
update/delete happening during the Scan phase and then essentially
no-op'ing the ExecForeignUpdate/ExecForeignDelete is entirely in-line
with the defined API.
Yeah, I've started looking at this patch and that seemed like not
necessarily such a wise choice. I think it'd be better if the generated
plan tree had a different structure in this case. The existing approach
is an impressive hack but it's hard to call it anything but a hack.
Thank you for the review, Tom and Stephen!
I guess that the idea is inspired by this comment on postgres_fdw.c:
* Note: currently, the plan tree generated for UPDATE/DELETE will always
* include a ForeignScan that retrieves ctids (using SELECT FOR UPDATE)
* and then the ModifyTable node will have to execute individual remote
* UPDATE/DELETE commands. If there are no local conditions or joins
* needed, it'd be better to let the scan node do UPDATE/DELETE RETURNING
* and then do nothing at ModifyTable. Room for future optimization ...
That's right. Thanks, Laurenz!
And in addition to that, I've created the patch with the conscious aim
of not getting the core code involved, because I was thinking this is
just an optimization. But I have to admit I was conscious of that too much.
I'm not sure offhand what the new plan tree ought to look like. We could
just generate a ForeignScan node, but that seems like rather a misnomer.
Is it worth inventing a new ForeignUpdate node type? Or maybe it'd still
be ForeignScan but with a flag field saying "hey this is really an update
(or a delete)". The main benefit I can think of right now is that the
EXPLAIN output would be less strange-looking --- but EXPLAIN is hardly
the only thing that ever looks at plan trees, so having an outright
misleading plan structure is likely to burn us down the line.
I can understand these qualms.
I wonder if "ForeignUpdate" is such a good name though, since it would
surprise the uninitiate that in the regular (no push-down) case the
actual modification is *not* performed by ForeignUpdate.
So it should rather be a "ForeignModifyingScan", but I personally would
prefer a "has_side_effects" flag on ForeignScan.
+1 for improving the EXPLAIN output by inventing a plan tree with a
different structure in this case, in general.
One advantage of getting the core code involved in the decision about
whether an update/delete can be pushed down is that FDW-independent checks
like whether there are relevant triggers could be implemented in the core
code, rather than having to duplicate them (and later maintain them) in
every FDW that wants to do this.
Good idea!
Stuff like "is there a
LIMIT" probably has to be in the FDW since some FDWs could support pushing
down LIMIT and others not.
That's what I have in mind. I'll work on that at the next CF.
Thanks,
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
