On Wed, May 8, 2019 at 5:05 PM Ashwin Agrawal <aagra...@pivotal.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.
