On Thu, Sep 21, 2023, at 12:53 PM, Etsuro Fujita wrote: > Hi, > > On Thu, Sep 21, 2023 at 11:49 AM Lepikhov Andrei > <a.lepik...@postgrespro.ru> wrote: >> On Wed, Sep 20, 2023, at 5:05 PM, Etsuro Fujita wrote: >> > What I am concerned about from the report [1] is that this comment is >> > a bit too terse; it might cause a misunderstanding that extensions can >> > do different things than we intend to allow: >> > >> > /* >> > * 6. Finally, give extensions a chance to manipulate the path list. >> > */ >> > if (set_join_pathlist_hook) >> > set_join_pathlist_hook(root, joinrel, outerrel, innerrel, >> > jointype, &extra); >> > >> > So I would like to propose to extend the comment to explain what they >> > can do, as in the comment about set_rel_pathlist_hook() in allpaths.c. >> > Attached is a patch for that. >> >> It makes sense. But why do you restrict addition to pathlist by only the >> add_path() routine? It can fail to add a path to the pathlist. We need to >> find out the result of the add_path operation and need to check the pathlist >> each time. So, sometimes, it can be better to add a path manually. > > I do not agree with you on this point; I think you can do so at your > own responsibility, but I think it is better for extensions to use > add_path(), because that makes them stable. (Assuming that add_path() > has a bug and we change the logic of it to fix the bug, extensions > that do not follow the standard procedure might not work anymore.)
Ok, I got it.This question related to the add_path() interface itself, not to the comment. It is awkward to check every time in the pathlist the result of the add_path. >> One more slip-up could be prevented by the comment: removing a path from the >> pathlist we should remember about the cheapest_* pointers. > > Do we really need to do this? I mean we do set_cheapest() afterward. > See standard_join_search(). Agree, in the case of current join it doesn't make sense. I stuck in this situation because providing additional path at the current level I need to arrange paths for the inner and outer too. Thanks for the explanation! -- Regards, Andrei Lepikhov
