Re: Cleaning up nbtree after logical decoding on standby work

On Fri, Jun 9, 2023 at 12:03 PM Andres Freund <andres@anarazel.de> wrote:
> 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 :/.

It's not 100% clear what the underlying principle is, but we mix
camelCase and underscore styles all the time, so that's always kinda
true.

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

Really? What parts are you opposed to in principle? I don't see why
you wouldn't support everything or nothing for 17 (questions of style
aside). I don't see what's ambiguous about what we should do here,
barring the 16-or-17 question.

It's not like nbtree ever really "used P_NEW". It doesn't actually
depend on any of the P_NEW handling inside bufmgr.c. It looks a little
like it might, but that's just an accident.

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

I don't see why, but okay. I'll do it that way.

--
Peter Geoghegan