Re: [PATCH] Incremental sort (was: PoC: Partial sort)

On Sat, Jul 20, 2019 at 11:25 AM Tomas Vondra
<tomas(dot)vondra(at)2ndquadrant(dot)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.

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.

James Coleman