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
