On Mon, Sep 16, 2019 at 6:32 AM Tomas Vondra <tomas.von...@2ndquadrant.com> wrote: > > On Sun, Sep 15, 2019 at 09:33:33PM -0400, James Coleman wrote: > >On Sat, Jul 20, 2019 at 11:25 AM Tomas Vondra > ><tomas.von...@2ndquadrant.com> wrote: > >> >> ... > >> >> > >> >> I think this may be a thinko, as this plan demonstrates - but I'm not > >> >> sure about it. I wonder if this might be penalizing some other types of > >> >> plans (essentially anything with limit + gather). > >> >> > >> >> Attached is a WIP patch fixing this by considering both startup and > >> >> total cost (by calling compare_path_costs_fuzzily). > >> > > >> >It seems to me that this is likely a bug, and not just a changed > >> >needed for this. Do you think it's better addressed in a separate > >> >thread? Or retain it as part of this patch for now (and possibly break > >> >it out later)? On the other hand, it's entirely possible that someone > >> >more familiar with parallel plan limitations could explain why the > >> >above comment holds true. That makes me lean towards asking in a new > >> >thread. > >> > > >> > >> Maybe. I think creating a separate thread would be useful, provided we > >> manage to demonstrate the issue without an incremental sort. > > > >I did some more thinking about this, and I can't currently come up > >with a way to reproduce this issue outside of this patch. It doesn't > >seem reasonable to me to assume that there's anything inherent about > >this patch that means it's the only way we can end up with a partial > >path with a low startup cost we'd want to prefer. > > > >Part of me wants to pull it over to a separate thread just to get > >additional feedback, but I'm not sure how useful that is given we > >don't currently have an example case outside of this patch. > > > > Hmm, I see. > > While I initially suggested to start a separate thread only if we have > example not involving an incremental sort, that's probably not a hard > requirement. I think it's fine to start a thead briefly explaining the > issue, and pointing to incremental sort thread for actual example. > > > > >One thing to note though: the current patch does not also modify > >add_partial_path_precheck which also does not take into account > >startup cost, so we probably need to update that for completeness's > >sake. > > > > Good point. It does indeed seem to make the same assumption about only > comparing total cost before calling add_path_precheck.
I've started a new thread to discuss: https://www.postgresql.org/message-id/CAAaqYe-5HmM4ih6FWp2RNV9rruunfrFrLhqFXF_nrrNCPy1Zhg%40mail.gmail.com James
