On Wed, Nov 4, 2020 at 11:32 AM Amit Langote <amitlangot...@gmail.com> wrote: > On Tue, Nov 3, 2020 at 9:05 PM Heikki Linnakangas <hlinn...@iki.fi> wrote: > > A) We could change FDW API so that BeginDirectModify takes the same > > arguments as BeginForeignModify(). That avoids the assumption that it's > > a ForeignScan node, because BeginForeignModify() doesn't take > > ForeignScanState as argument. That would be consistent, which is nice. > > But I think we'd somehow still need to pass the ResultRelInfo to the > > corresponding ForeignScan, and I'm not sure how. > > Maybe ForeignScan doesn't need to contain any result relation info > then? ForeignScan.operation != CMD_SELECT is enough to tell it to > call IterateDirectModify() as today. > > > B) Look up the ResultRelInfo, and call BeginDirectModify(), on the first > > call to ForeignNext(). > > > > C) Accept the Assertion. And add an elog() check in the planner for that > > with a proper error message. > > > > I'm leaning towards B), but maybe there's some better solution I didn't > > think of? Perhaps changing the API would make sense in any case, it is a > > bit weird as it is. Backwards-incompatible API changes are not nice, but > > I don't think there are many FDWs out there that implement the > > DirectModify functions. And those functions are pretty tightly coupled > > with the executor and ModifyTable node details anyway, so I don't feel > > like we can, or need to, guarantee that they stay unchanged across major > > versions. > > B is not too bad, but I tend to prefer doing A too.
How about I update the 0001 patch to implement A? -- Amit Langote EDB: http://www.enterprisedb.com
