Re: [PATCH] Use optimized single-datum tuplesort in ExecSort
От | Ronan Dunklau |
---|---|
Тема | Re: [PATCH] Use optimized single-datum tuplesort in ExecSort |
Дата | |
Msg-id | 6987360.ZhFIMvCLOo@aivenronan обсуждение исходный текст |
Ответ на | Re: [PATCH] Use optimized single-datum tuplesort in ExecSort (David Rowley <dgrowleyml@gmail.com>) |
Ответы |
Re: [PATCH] Use optimized single-datum tuplesort in ExecSort
|
Список | pgsql-hackers |
Le lundi 12 juillet 2021, 15:11:17 CEST David Rowley a écrit : > On Wed, 7 Jul 2021 at 21:32, Ronan Dunklau <ronan.dunklau@aiven.io> wrote: > > In the meantime I fixed some formatting issues, please find attached a new > > patch. > > I started to look at this. Thank you ! I'm attaching a new version of the patch taking your remarks into account. > > First I wondered how often we might be able to apply this > optimisation, so I ran make check after adding some elog(NOTICE) calls > to output which method is going to be used just before we do the > tuplestore_begin_* calls. It looks like there are 614 instances of > Datum sorts and 4223 of tuple sorts. That's about 14.5% datum sorts. > 223 of the 614 are byval types and the other 391 are byref. Not that > the regression tests are a good reflection of the real world, but if > it were then that's quite a good number of cases to be able to > optimise. That's an interesting stat. > > As for the patch, just a few things: > > 1. Can you add the missing braces in this if condition and the else > condition that belongs to it. > > + if (node->is_single_val) > + for (;;) > + { > Done. > 2. I think it would nicer to name the new is_single_val field > "datumSort" instead. To me it seems more clear what it is for. Done. > > 3. This seems to be a bug fix where byval datum sorts do not properly > handle bounded sorts. I think that maybe that should be fixed and > backpatched. I don't see anything that says Datum sorts can't be > bounded and if there were some restriction on that I'd expect > tuplesort_set_bound() to fail when the Tuplesortstate had been set up > with tuplesort_begin_datum(). I've kept this as-is for now, but I will remove it from my patch if it is deemed worthy of back-patching in your other thread. Regards, -- Ronan Dunklau
Вложения
В списке pgsql-hackers по дате отправления: