Re: Unhappy about API changes in the no-fsm-for-small-rels patch

On Wed, May 1, 2019 at 9:57 AM John Naylor <john(dot)naylor(at)2ndquadrant(dot)com> wrote:
>
> On Tue, Apr 30, 2019 at 12:48 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Fri, Apr 26, 2019 at 10:46 AM John Naylor
> > <john(dot)naylor(at)2ndquadrant(dot)com> wrote:
> > I don't much like the new function name GetAlternatePage, may be
> > GetPageFromLocalFSM or something like that. OTOH, I am not sure if we
> > should go that far to address this concern of Andres's, maybe just
> > adding a proper comment is sufficient.
>
> That's a clearer name. I think 2 functions is easier to follow than
> the boolean parameter.
>

Okay, but then add a few comments where you are calling that function.

> > > Putting the thresholds in 3 files with completely different purposes
> > > is a mess, and serves no example for future access methods, but I
> > > don't have a better idea.
> > >
> >
> > Yeah, I am also not sure if it is a good idea because it still won't
> > be easy for pluggable storage especially the pg_upgrade part. I think
> > if we really want to make it easy for pluggable storage to define
> > this, then we might need to build something along the lines of how to
> > estimate relation size works.
> >
> > See how table_relation_estimate_size is defined and used
> > and TableAmRoutine heapam_methods
> > {
> > ..
> > relation_estimate_size
> > }
>
> That might be the best way for table ams, but I guess we'd still need
> to keep the hard-coding for indexes to always have a FSM. That might
> not be too bad.
>

I also think so.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com