Re: Add proper planner support for ORDER BY / DISTINCT aggregates - Mailing list pgsql-hackers

From Ranier Vilela
Subject Re: Add proper planner support for ORDER BY / DISTINCT aggregates
Date
Msg-id CAEudQArZmwkahm7pjy3_nKn2CRBZ1Dhf4qtT+J4wkdVUD8yfpA@mail.gmail.com
Whole thread Raw
In response to Add proper planner support for ORDER BY / DISTINCT aggregates  (David Rowley <dgrowleyml@gmail.com>)
Responses Re: Add proper planner support for ORDER BY / DISTINCT aggregates  (Ronan Dunklau <ronan.dunklau@aiven.io>)
List pgsql-hackers

>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:

1. Why do you moved the declaration of variable *plannode?
I think this is unnecessary, extend the scope.

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.

3. I inverted the order of check at this line, I think "!node-bounded" is more cheap that TupleDescAttr(tupDesc, 0) ->attbyval

4. Once that you changed the order of execution, this test is unlikely that happens, so add unlikely helps the branch.

5. I think that you add a invariant inside the loop "if(node->is_single_val)"?
Would not be better two fors?

For you convenience, I attached a v2 version (with styles changes), please take a look and can you repeat yours tests?

regards,
Ranier Vilela
Attachment

pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: Disable WAL logging to speed up data loading
Next
From: Ronan Dunklau
Date:
Subject: Re: Add proper planner support for ORDER BY / DISTINCT aggregates