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.
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 ... > 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. > 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. OTOH, maybe the trigger issue is really > the only thing that could be shared, not sure. Stuff like "is there a > LIMIT" probably has to be in the FDW since some FDWs could support pushing > down LIMIT and others not. You are right, the gain would probably be limited. Yours, Laurenz Albe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
