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

On Thu, Apr 24, 2025 at 5:40 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> Yeah. Quoting the comments for outersortkeys/innersortkeys:
>
> * outersortkeys (resp. innersortkeys) is NIL if the outer path
> * (resp. inner path) is already ordered appropriately for the
> * mergejoin. If it is not NIL then it is a PathKeys list describing
> * the ordering that must be created by an explicit Sort node.
>
> And try_mergejoin_path will detect whether the outer path (resp. inner
> path) is already well enough ordered, and suppresses an explicit sort
> step if so by setting outersortkeys (resp. innersortkeys) to NIL.
>
> This reflects a basic assumption: if MergePath.outersortkeys is not
> NIL, it means the outer path is not sufficiently ordered.
>
> Therefore, I think the "Assert(!is_sorted)" when outersortkeys is not
> NIL is reasonable. And I think some other code is violating this
> assumption.
>
> I tried this repro query on v17 and got a plan with a redundant Sort
> node. It seems that this issue existed prior to commit 828e94c9d, and
> the Assert introduced in that commit makes the issue easier to detect.

It seems the culprit is in GetExistingLocalJoinPath(), which is used
to get a local path for EPQ checks. In that function, if the outer or
inner path of the selected join path is a ForeignScan, it is replaced
with the fdw_outerpath.

The problem arises when the selected join path is a MergePath, and its
outer or inner path is a ForeignScan that is not already well enough
ordered. In this case, the MergePath will have non-NIL outersortkeys
or innersortkeys. If we then replace the outer or inner path with the
corresponding fdw_outerpath, and that path is already sufficiently
ordered, we end up in an inconsistent state: the MergePath has non-NIL
outersortkeys/innersortkeys, and its input path is already properly
ordered.

This inconsistency leads to a redundant Sort node prior to commit
828e94c9d, and triggers an Assert failure after that commit.

Thanks
Richard