Em sex., 16 de jul. de 2021 às 00:45, David Rowley <dgrowleyml@gmail.com> escreveu:
On Fri, 16 Jul 2021 at 02:53, Ronan Dunklau <ronan.dunklau@aiven.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.