Re: [PATCH] Use optimized single-datum tuplesort in ExecSort

Em sex., 16 de jul. de 2021 às 00:45, David Rowley <dgrowleyml(at)gmail(dot)com>
escreveu:

> On Fri, 16 Jul 2021 at 02:53, Ronan Dunklau <ronan(dot)dunklau(at)aiven(dot)io>
> wrote:
> > Please find attached a v9 just moving the flag setting to ExecInitSort,
> and my
> > apologies if I misunderstood your point.
>
> I took this and adjusted a few things and ended up with the attached patch.
>
> The changes are fairly minor. I made the bracing consistent between
> both tuplesort_begin calls. I rewrote the comment at the top of
> ExecSort() to make it more clear about each method used.
>
With relation to the braces, it's still not clear to me which style to
follow.
I gave Ronan directions about it.
And I think maybe, it's still not clear when to use it or not.

> I also adjusted the comment down at the end of ExecSort that was
> mentioning something about tuplesort_gettupleslot returning NULL.
> Your patch didn't touch this, but to me, the comment just looked wrong
> both before and after the changes. tuplesort_gettupleslot returns
> false and sets the slot to empty when it runs out of tuples. Anyway,
> I wrote something there that I think improves that.
>
Can help a little here, but, seems good to me.

> I feel like this patch is commit-worthy now. However, I'll leave it
> for a few days, maybe until after the weekend as there's been a fair
> bit of interest and I imagine someone will have comments to make.
>
A little lack of time.

But I finally can understand v7b.
Really struct field is necessary and he fails with the next tuple, ok.
The only conclusion I can come to is that he is faster because he fails to
sort correctly.
It's no use being faster and getting wrong results.

So, +1 from me to commit v10.

Thanks for working together.

Ranier Vilela