Le lundi 5 juillet 2021, 16:51:59 CEST Ranier Vilela a écrit : > >Please find attached a POC patch to do just that. > > > >The switch to the single-datum tuplesort is done when there is only one > >attribute, it is byval (to avoid having to deal with copy of the > > references > > >everywhere) and we are not in bound mode (to also avoid having to move > > things > > >around). > > Hi, nice results! > > I have a few suggestions and questions to your patch:
Thank you for those ! > > 1. Why do you moved the declaration of variable *plannode? > I think this is unnecessary, extend the scope.
Sorry, I should have cleaned it up before sending.
> > 2. Why do you declare a new variable TupleDesc out_tuple_desc at > ExecInitSort? > I think this is unnecessary too, maybe I didn't notice something.
Same as the above, thanks for the two. > > 3. I inverted the order of check at this line, I think "!node-bounded" is > more cheap that TupleDescAttr(tupDesc, 0) ->attbyval
I'm not sure it matters since it's done once per sort but Ok > > 4. Once that you changed the order of execution, this test is unlikely that > happens, so add unlikely helps the branch.
Ok.
> > 5. I think that you add a invariant inside the loop > "if(node->is_single_val)"? > Would not be better two fors?
Ok for me.
> > For you convenience, I attached a v2 version (with styles changes), please > take a look and can you repeat yours tests?
Tested it quickly, and did not see any change performance wise that cannot be attributed to noise on my laptop but it's fine.