Re: Inconsistency between table am callback and table function names

Hi,

On 2019-05-08 00:32:22 -0700, Ashwin Agrawal wrote:
> The general theme for table function names seem to be
> "table_<am_callback_name>". For example table_scan_getnextslot() and its
> corresponding callback scan_getnextslot(). Most of the table functions and
> callbacks follow mentioned convention except following ones
>
> table_beginscan
> table_endscan
> table_rescan
> table_fetch_row_version
> table_get_latest_tid
> table_insert
> table_insert_speculative
> table_complete_speculative
> table_delete
> table_update
> table_lock_tuple
>
> the corresponding callback names for them are
>
> scan_begin
> scan_end
> scan_rescan

The mismatch here is just due of backward compat with the existing
function names.

> tuple_fetch_row_version
> tuple_get_latest_tid

Hm, I'd not object to adding a tuple_ to the wrapper.

> tuple_insert
> tuple_insert_speculative
> tuple_delete
> tuple_update
> tuple_lock

That again is to keep the naming similar to the existing functions.

> Also, some of these table function names read little odd
>
> table_relation_set_new_filenode
> table_relation_nontransactional_truncate
> table_relation_copy_data
> table_relation_copy_for_cluster
> table_relation_vacuum
> table_relation_estimate_size
>
> Can we drop relation word from callback names and as a result from these
> function names as well? Just have callback names as set_new_filenode,
> copy_data, estimate_size?

I'm strongly against that. These all work on a full relation size,
rather than on individual tuples, and that seems worth pointing out.

> Also, a question about comments. Currently, redundant comments are written
> above callback functions as also above table functions. They differ
> sometimes a little bit on descriptions but majority content still being the
> same. Should we have only one place finalized to have the comments to keep
> them in sync and also know which one to refer to?

Note that the non-differing comments usually just refer to the other
place. And there's legitimate differences in most of the ones that are
both at the callback and the external functions - since the audience of
both are difference, that IMO makes sense.

> Plus, file name amapi.h now seems very broad, if possible should be renamed
> to indexamapi.h or indexam.h to follow tableam.h. No idea what's our policy
> around header file renames.

We probably should rename it, but not in 12...

Greetings,

Andres Freund