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