Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"

On Thu, 24 Apr 2025 at 21:44, Richard Guo <guofenglinux@gmail.com> wrote:
> One problem with this approach is that the cost of the explicit sort
> has already been included in the MergePath.  However, I'm not too
> concerned about this, since the resulting plan is only used for EPQ
> checks, where cost accuracy is not that important.  After all, we also
> don't adjust the join path's cost when replacing its outer or inner
> subpath.
>
> Any thoughts?

One thing I noticed when looking at this is the number of redundant
pathkey checks we have now due to 828e94c9d.

In try_mergejoin_path(), we first call pathkeys_contained_in() on
either side of the join and nullify the pathkeys if they're presorted.
If they're not presorted then initial_cost_mergejoin() checks again by
calling pathkeys_count_contained_in() so that we can figure out the
sort costs. If that MergePath wins the path battle, then
create_mergejoin_plan() will check the pathkeys for the 3rd time with
pathkeys_count_contained_in() to figure out what type of sort needs to
happen.

I think MergePath needs to do a bit more caching of sorted-ness
information. How about making MergePath remember how many leading
PathKeys the inputs paths are sorted on. If we had that then 0 could
mean "not sorted" rather than a NIL MergePath.outersortkeys /
innersortkeys field. In fact, is there any point in having MergePath
outersortkeys and innersortkeys at all? Couldn't
create_mergejoin_plan() look at the outer_path->pathkeys for
make_sort_from_pathkeys/make_incrementalsort_from_pathkeys? We could
probably get rid of a few dozen lines of code by doing this and
there'd be less concern that something gets out of sync as everything
would just trust that the npresorted_outerkeys is correct. That would
be calculated in try_partial_mergejoin_path() before calling
initial_cost_mergejoin().

Looking at the calendar... it's still April. I think we should make
this better for v18. I don't see the sense in v18 being the only
version where it works this way.

David