Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)

On Sun, Feb 26, 2023 at 1:52 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wrote:
> > The issue seems to be that code like this:
> > ...
> > is far too cute for its own good.
>
> Oh, there's another thing here that qualifies as too-cute: loops like
>
>     for (IOObject io_object = IOOBJECT_FIRST;
>          io_object < IOOBJECT_NUM_TYPES; io_object++)
>
> make it look like we could define these enums as 1-based rather
> than 0-based, but if we did this code would fail, because it's
> confusing "the number of values" with "1 more than the last value".
>
> Again, we could fix that with tests like "io_context <= IOCONTEXT_LAST",
> but I don't see the point of adding more macros rather than removing
> some.  We do need IOOBJECT_NUM_TYPES to declare array sizes with,
> so I think we should nuke the "xxx_FIRST" macros as being not worth
> the electrons they're written on, and write these loops like
>
>     for (int io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
>
> which is not actually adding any assumptions that you don't already
> make by using io_object as a C array subscript.

Attached is a patch to remove the *_FIRST macros.
I was going to add in code to change

    for (IOObject io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
    to
    for (IOObject io_object = 0; (int) io_object < IOOBJECT_NUM_TYPES;
io_object++)

but then I couldn't remember why we didn't just do

    for (int io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)

I recall that when passing that loop variable into a function I was
getting a compiler warning that required me to cast the value back to an
enum to silence it:

            pgstat_tracks_io_op(bktype, (IOObject) io_object,
io_context, io_op))

However, I am now unable to reproduce that warning.
Moreover, I see in cases like table_block_relation_size() with
ForkNumber, the variable i is passed with no cast to smgrnblocks().

- Melanie