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

On Wed, Apr 23, 2025 at 8:47 PM Tender Wang <tndrwang@gmail.com> wrote:
>>                      ->  Merge Join
>>                            Merge Cond: (ft1.c2 = ft5.c1)
>>                            ->  Sort
>>                                  Sort Key: ft1.c2
>>                                  ->  Merge Join
>>                                        Merge Cond: (ft1.c2 = ft4.c1)
>>                                        ->  Sort
>>                                              Sort Key: ft1.c2

This plan seems problematic to me.  The Sort node above the MergeJoin
of ft1/ft4 is redundant, as the output of the MergeJoin is already
ordered by ft1.c2.

> If is_sorted is true, it means that it's already fully sorted,  the Sort node doesn't need any more.
> I suspect something went wrong somewhere else.  I didn't look into the details.

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.

Thanks
Richard