From: | Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> |
---|---|
To: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Cc: | ronan(dot)dunklau(at)aiven(dot)io, David Rowley <dgrowleyml(at)gmail(dot)com> |
Subject: | Re: Add proper planner support for ORDER BY / DISTINCT aggregates |
Date: | 2021-07-05 14:51:59 |
Message-ID: | CAEudQArZmwkahm7pjy3_nKn2CRBZ1Dhf4qtT+J4wkdVUD8yfpA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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 | Content-Type | Size |
---|---|---|
v2-0001-Allow-Sort-nodes-to-use-the-fast-single-datum-tuples.patch | application/octet-stream | 3.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Ronan Dunklau | 2021-07-05 15:07:27 | Re: Add proper planner support for ORDER BY / DISTINCT aggregates |
Previous Message | Stephen Frost | 2021-07-05 13:57:41 | Re: Disable WAL logging to speed up data loading |