Re: Inconsistency between table am callback and table function names

Hi,

On 2019-05-10 12:43:06 -0700, Ashwin Agrawal wrote:
> On Fri, May 10, 2019 at 10:51 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> >
> > Hi,
> >
> > On 2019-05-10 10:43:44 -0700, Ashwin Agrawal wrote:
> > > On Thu, May 9, 2019 at 8:52 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > > > The changes necessary for tableam were already huge. Changing naming
> > > > schemes for functions that are used all over the backend (e.g. ~80 calls
> > > > to table_beginscan), and where there's other wrapper functions that also
> > > > widely used (237 calls to systable_beginscan) which didn't have to be
> > > > touched, at the same time would have made it even harder to review.
> > >
> > > If there are no objections to renaming now, as separate independent
> > > patch, I am happy to do the same and send it across. Will rename to
> > > make it consistent as mentioned at start of the thread leaving
> > > table_relation_xxx() ones as is today.
> >
> > What would you want to rename precisely? Don't think it's useful to
> > start sending patches before we agree on something concrete. I'm not on
> > board with patching hundreds systable_beginscan calls (that'll break a
> > lot of external code, besides the churn of in-core code), nor with the
> > APIs around that having a diverging name scheme.
>
> Meant to stick the question mark in that email, somehow missed. Yes
> not planning to spend any time on it if objections. Here is the list
> of renames I wish to perform.
>
> Lets start with low hanging ones.
>
> table_rescan -> table_scan_rescan
> git grep table_rescan | wc -l
> 6
>
> table_insert -> table_tuple_insert
> git grep tuple_insert | wc -l
> 13
>
> table_insert_speculative -> table_tuple_insert_speculative
> git grep tuple_insert_speculative | wc -l
> 5
>
> table_delete -> table_tuple_delete (table_delete reads incorrect as
> not deleting the table)
> git grep tuple_delete | wc -l
> 8
>
> table_update -> table_tuple_update
> git grep tuple_update | wc -l
> 5
>
> table_lock_tuple -> table_tuple_lock
> git grep tuple_lock | wc -l
> 26
>
>
> Below two you already mentioned no objections to rename
> table_fetch_row_version -> table_tuple_fetch_row_version
> table_get_latest_tid -> table_tuple_get_latest_tid
>
>
> Now, table_beginscan and table_endscan are the ones which are
> wide-spread. Desire seems we should keep it consistent with
> systable_beginscan. Understand the constraints and churn aspect, given
> that diverging naming scheme is unavoidable. Why not leave
> systable_beginscan as it is and only rename table_beginscan and its
> counterparts table_beginscan_xxx() atleast?
>
> Index interfaces and table interfaces have some diverged naming scheme
> already like index_getnext_slot and table_scan_getnextslot. Not
> proposing to change them. But at least reducing wherever possible
> sooner would be helpful.

My personal opinion is that this is more churn than I think is useful to
tackle after feature freeze, with not sufficient benefits. If others
chime in, voting to do this, I'm OK with doing that, but otherwise I
think there's more important stuff to do.

Greetings,

Andres Freund