Hi, On 2019-08-03 05:20:35 +0900, Etsuro Fujita wrote: > On Sat, Aug 3, 2019 at 3:01 AM Andres Freund <and...@anarazel.de> wrote: > > On 2019-07-31 17:04:38 +0900, Amit Langote wrote: > > > 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. > > > Fujita-san, do you have any comments on the FDW API change? Or anybody > > else? > > > > I'm a bit woried about the move of BeginDirectModify() into > > nodeModifyTable.c - it just seems like an odd control flow to me. Not > > allowing any intermittent nodes between ForeignScan and ModifyTable also > > seems like an undesirable restriction for the future. I realize that we > > already do that for BeginForeignModify() (just btw, that already accepts > > resultRelInfo as a parameter, so being symmetrical for BeginDirectModify > > makes sense), but it still seems like the wrong direction to me. > > > > The need for that move, I assume, comes from needing knowing the correct > > ResultRelInfo, correct? I wonder if we shouldn't instead determine the > > at plan time (in setrefs.c), somewhat similar to how we determine > > ModifyTable.resultRelIndex. Doesn't look like that'd be too hard? > > I'd vote for that; I created a patch for that [1]. > > [1] > https://www.postgresql.org/message-id/CAPmGK15%3DoFHmWNND5vopfokSGfn6jMXVvnHa7K7P49F7k1hWPQ%40mail.gmail.com
Oh, missed that. But that's not quite what I'm proposing. I don't like ExecFindResultRelInfo at all. What's the point of it? It's introduction is still an API break - I don't understand why that break is better than just passing the ResultRelInfo directly to BeginDirectModify()? I want to again remark that BeginForeignModify() does get the ResultRelInfo - it should have been done the same when adding direct modify. Even if you need the loop - which I don't think is right - it should live somewhere that individual FDWs don't have to care about. - Andres
