Re: Inconsistency between table am callback and table function names

On Wed, May 8, 2019 at 5:05 PM Ashwin Agrawal <aagrawal(at)pivotal(dot)io> wrote:

> Not having consistency is the main aspect I wish to bring to
> attention. Like for some callback functions the comment is
>
> /* see table_insert() for reference about parameters */
> void (*tuple_insert) (Relation rel, TupleTableSlot *slot,
> CommandId cid, int options,
> struct BulkInsertStateData *bistate);
>
> /* see table_insert_speculative() for reference about parameters
> */
> void (*tuple_insert_speculative) (Relation rel,
> TupleTableSlot *slot,
> CommandId cid,
> int options,
> struct
> BulkInsertStateData *bistate,
> uint32 specToken);
>
> Whereas for some other callbacks the parameter explanation exist in
> both the places. Seems we should be consistent.
> I feel in long run becomes pain to keep them in sync as comments
> evolve. Like for example
>
> /*
> * Estimate the size of shared memory needed for a parallel scan
> of this
> * relation. The snapshot does not need to be accounted for.
> */
> Size (*parallelscan_estimate) (Relation rel);
>
> parallescan_estimate is not having snapshot argument passed to it, but
> table_parallescan_estimate does. So, this way chances are high they
> going out of sync and missing to modify in both the places. Agree
> though on audience being different for both. So, seems going with the
> refer XXX for parameters seems fine approach for all the callbacks and
> then only specific things to flag out for the AM layer to be aware can
> live above the callback function.
>

The topic of consistency for comment style for all wrappers seems got lost
with other discussion, would like to seek opinion on the same.