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

On Thu, Apr 24, 2025 at 7:58 PM David Rowley <dgrowleyml@gmail.com> wrote:
> 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().

Hmm, well, I don't think we can get rid of MergePath.outersortkeys and
innersortkeys, even if we've cached the number of presorted keys for
the outer and inner paths.  These fields represent the desired
ordering to be produced by an explicit Sort node — not the current
ordering of the outer or inner path.  Without this information, how
would create_mergejoin_plan know what sort order to generate for the
outer and inner paths if they are not already sorted?

Maybe we could cache the number of presorted keys for the outer and
inner paths in MergePath to save some pathkeys_count_contained_in
calls.  But I'm not too sure it's worthwhile.  The pathkeys are
canonical, and can be checked for equality by simple pointer
comparison.  So it does not seem to cost too much.  Besides, the
"redundant" pathkey checks actually helped uncover the bug we're
discussing here — didn't they?

Thanks
Richard