Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)

Hello Rushabh,

On Fri, December 8, 2017 2:28 am, Rushabh Lathia wrote:
> Thanks for review.
>
> On Fri, Dec 8, 2017 at 6:27 AM, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
>> On Thu, Dec 7, 2017 at 12:25 AM, Rushabh Lathia
>> <rushabh(dot)lathia(at)gmail(dot)com> wrote:
>> > 0001-Add-parallel-B-tree-index-build-sorting_v14.patch

I've looked only at patch 0002, here are some comments.

> + * leaderasworker indicates whether leader going to participate as
worker or
> + * not.

The grammar is a bit off, and the "or not" seems obvious. IMHO this could be:

+ * leaderasworker indicates whether the leader is going to participate as
worker

The argument leaderasworker is only used once and for one temp. variable
that is only used once, too. So the temp. variable could maybe go.

And not sure what the verdict was from the const-discussion threads, I did
not follow it through. If "const" is what should be done generally, then
the argument could be consted, as to not create more "unconsted" code.

E.g. so:

+plan_create_index_workers(Oid tableOid, Oid indexOid, const bool
leaderasworker)

and later:

- sort_mem_blocks / (parallel_workers + 1) <
min_sort_mem_blocks)
+ sort_mem_blocks / (parallel_workers + (leaderasworker
? 1 : 0)) < min_sort_mem_blocks)

Thank you for working on this patch!

All the best,

Tels