On Wed, Jul 31, 2019 at 5:05 PM Amit Langote <amitlangot...@gmail.com> wrote: > On Tue, Jul 30, 2019 at 4:20 PM Amit Langote <amitlangot...@gmail.com> wrote: > > On Sat, Jul 20, 2019 at 1:52 AM Andres Freund <and...@anarazel.de> wrote: > > > IOW, we should just change the direct modify calls to get the relevant > > > ResultRelationInfo or something in that vein (perhaps just the relevant > > > RT index?). > > > > It seems easy to make one of the two functions that constitute the > > direct modify API, IterateDirectModify(), access the result relation > > from ForeignScanState by saving either the result relation RT index or > > ResultRelInfo pointer itself into the ForeignScanState's FDW-private > > area. For example, for postgres_fdw, one would simply add a new > > member to PgFdwDirectModifyState struct. > > > > Doing that for the other function BeginDirectModify() seems a bit more > > involved. We could add a new field to ForeignScan, say > > resultRelation, that's set by either PlanDirectModify() (the FDW code) > > or make_modifytable() (the core code) if the ForeignScan node contains > > the command for direct modification. BeginDirectModify() can then use > > that value instead of relying on es_result_relation_info being set. > > > > Thoughts? Fujita-san, do you have any opinion on whether that would > > be a good idea?
I'm still not sure that it's a good idea to remove es_result_relation_info, but if I had to say then I think the latter would probably be better. I'm planning to rework on direct modification to base it on upper planner pathification so we can perform direct modification without the ModifyTable node. (I'm not sure we can really do this for inherited UPDATE/DELETE, though.) For that rewrite, I'm thinking to call BeginDirectModify() from the ForeignScan node (ie, ExecInitForeignScan()) as-is. The latter approach would allow that without any changes and avoid changing that API many times. That's the reason why I think the latter would probably be better. > I looked into trying to do the things I mentioned above and it seems > to me that revising BeginDirectModify()'s API to receive the > ResultRelInfo directly as Andres suggested might be the best way > forward. I've implemented that in the attached 0001. Patches that > were previously 0001, 0002, and 0003 are now 0002, 003, and 0004, > respectively. 0002 is now a patch to "remove" > es_result_relation_info. Sorry for speaking this late. Best regards, Etsuro Fujita
