Hi,
On 2023-06-08 08:50:31 -0700, Peter Geoghegan wrote:
> On Thu, Jun 8, 2023 at 7:22 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > IMO this kind of change definitely does not have place in a post-beta1
> > restructuring patch. We rarely indulge in case-fixing exercises at any
> > other time, and I don't see any good argument why post-beta1 is a better
> > time for it.
>
> There is a glaring inconsistency. Now about half of the relevant
> functions in nbtree.h use "heaprel", while the other half use
> "heapRel". Obviously that's not the end of the world, but it's
> annoying. It's exactly the kind of case-fixing exercise that does tend
> to happen.
From what I can tell it's largely consistent with other parameters of the
respective function. E.g. btinsert(), _bt_doinsert() use camelCase for most
parameters, so heapRel fits in. There are a few cases where it's not obvious
what the pattern is intended to be :/.
> My new plan is to commit this tomorrow, since the clear consensus is
> that we should go ahead with this for 16.
I'm not sure there is that concensus (for me half the changes shouldn't be
done, the rest should be in 17), but in the end it doesn't matter that much.
> --- a/src/include/utils/tuplesort.h
> +++ b/src/include/utils/tuplesort.h
> @@ -399,9 +399,7 @@ extern Tuplesortstate *tuplesort_begin_heap(TupleDesc tupDesc,
> int workMem, SortCoordinate coordinate,
> int sortopt);
> extern Tuplesortstate *tuplesort_begin_cluster(TupleDesc tupDesc,
> - Relation indexRel,
> - Relation heaprel,
> - int workMem,
> + Relation indexRel, int workMem,
> SortCoordinate coordinate,
> int sortopt);
I think we should continue to provide the table here, even if we don't need it
today.
Greetings,
Andres Freund