Re: Remove all "INTERFACE ROUTINES" style comments - Mailing list pgsql-hackers

On 12/01/2019 03:12, Andres Freund wrote:
> On 2019-01-10 15:58:41 -0800, Andres Freund wrote:
>> A number of postgres files have sections like heapam's
>>
>>   * INTERFACE ROUTINES
>> ...
>> They're often out-of-date, and I personally never found them to be
>> useful. A few people, including yours truly, have been removing a few
>> here and there when overhauling a subsystem to avoid having to update
>> and then adjust them.
>>
>> I think it might be a good idea to just do that for all at once. Having
>> to consider separately committing a removal, updating them without
>> fixing preexisting issues, or just leaving them outdated on a regular
>> basis imo is a usless distraction.
> 
> As the reaction was positive, here's a first draft of a commit removing
> them. A few comments:
> 
> - I left two INTERFACE ROUTINES blocks intact, because they actually add
>    somewhat useful information. Namely fd.c's, which actually seems
>    useful, and predicate.c's about which I'm less sure.
> - I tried to move all comments about the routines in the INTERFACE
>    section to the functions if they didn't have a roughly equivalent
>    comment. Even if the comment wasn't that useful. Particularly just
>    about all the function comments in executor/node*.c files are useless,
>    but I thought that's something best to be cleaned up separately.
> - After removing the INTERFACE ROUTINES blocks a number of executor
>    files had a separate comment block with just a NOTES section. I merged
>    these with the file header comment blocks, and indented them to
>    match. I think this is better, but I'm only like 60% convinced of
>    that.
> 
> Comments? I'll revisit this patch on Monday or so, make another pass
> through it, and push it then.

I agree that just listing all the public functions in a source file is 
not useful. But listing the most important ones, perhaps with examples 
on how to use them together, or grouping functions when there are a lot 
of them, is useful. A high-level view of the public interface is 
especially useful for people who are browsing the code for the first time.

The comments in execMain.c seemed like a nice overview to the interface. 
I'm not sure the comments on each function do quite the same thing. The 
grouping of the functions in pqcomm.c's seemed useful. Especially when 
some of the routines listed there are actually macros defined in 
libpq.h, so if someone just looks at the contents of pqcomm.c, he might 
not realize that there's more in libpq.h. The grouping in pqformat.c 
also seemd useful.

In that vein, the comments in heapam.c could be re-structured, something 
like this:

  * Opening/closing relations
  * -------------------------
  *
  * The relation_* functions work on any relation, not only heap
  * relations:
  *
  *  relation_open   - open any relation by relation OID
  *  relation_openrv - open any relation specified by a RangeVar
  *  relation_close  - close any relation
  *
  * These are similar, but check that the relation is a Heap
  * relation:
  *
  *  heap_open        - open a heap relation by relation OID
  *  heap_openrv      - open a heap relation specified by a RangeVar
  *  heap_close       - (now just a macro for relation_close)
  *
  * Heap scans
  * ----------
  *
  * Functions for doing a Sequential Scan on a heap table:
  *
  *  heap_beginscan      - begin relation scan
  *  heap_rescan            - restart a relation scan
  *  heap_endscan        - end relation scan
  *  heap_getnext        - retrieve next tuple in scan
  *
  * To retrieve a single heap tuple, by tid:
  *  heap_fetch          - retrieve tuple with given tid
  *
  * Updating a heap
  * ---------------
  *
  *  heap_insert         - insert tuple into a relation
  *  heap_multi_insert   - insert multiple tuples into a relation
  *  heap_delete         - delete a tuple from a relation
  *  heap_update         - replace a tuple in a relation with another tuple
  *  heap_sync           - sync heap, for when no WAL has been written

- Heikki