Re: [PROPOSAL] : Use of ORDER BY clause in insert.sql

On Fri, 28 Oct 2022 at 16:51, Amul Sul <sulamul(at)gmail(dot)com> wrote:
>
> On Thu, Oct 27, 2022 at 6:54 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Please be specific about the circumstances in which the output is
> > unstable for you. With zero information to go on, it seems about as
> > likely that this change is masking a bug as that it's a good idea.
> >
>
> At the first glance, I thought the patch is pretty much obvious, and
> we usually add an ORDER BY clause to ensure stable output.

Unfortunately, you'll need to do better than that. We're not in the
business of accepting patches with zero justification for why they're
required. If you're not willing to do the analysis on why the order
changes sometimes, why should we accept your patch?

If you can't find the problem then you should modify insert.sql to
EXPLAIN the problem query to see if the plan has changed between the
passing and failing run. The only thing that comes to mind about why
this test might produce rows in a different order would be if a
parallel Append was sorting the subpaths by cost (See
create_append_path's call to list_sort) and the costs were for some
reason coming out differently sometimes. It's hard to imagine why this
query would be parallelised though. If you show us the EXPLAIN from a
passing and failing run, it might help us see the problem.

> If we
> are too sure that the output usually comes in the same order then the
> ORDER BY clause that exists in other tests seems useless. I am a bit
> confused & what could be a possible bug?

You can't claim that if this test shouldn't get an ORDER BY that all
tests shouldn't have an ORDER BY. That's just crazy. What if the test
is doing something like testing sort?!

David