From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, melanieplageman@gmail.com,
        vignesh21@gmail.com, pryzby@telsasoft.com, lukas@fittl.com,
        alvherre@alvh.no-ip.org, magnus@hagander.net,
        pgsql-hackers@postgresql.org, thomas.munro@gmail.com,
        m.sakrejda@gmail.com
Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
In-reply-to: <20230209050319.chyyup4vtq4jzobq@awork3.anarazel.de>
References: <CAAKRu_af0YcV9CXG9M5R06tYRUzk13+H7qOsU6etmS4zE7RAUQ@mail.gmail.com> <20230124.172203.448918351330829200.horikyota.ntt@gmail.com> <20230124223512.xqp7tjgqvzqokaxq@awork3.anarazel.de> <20230125.165617.724951472557617342.horikyota.ntt@gmail.com> <20230208063814.boyh5ibydbfuo2uk@awork3.anarazel.de> <20230209050319.chyyup4vtq4jzobq@awork3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Wed, 08 Feb 2023 21:03:19 -0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <20519.1677435600.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sun, 26 Feb 2023 13:20:00 -0500
Message-ID: <20520.1677435600@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/20520.1677435600%40sss.pgh.pa.us>
Precedence: bulk

Andres Freund <andres@anarazel.de> writes:
> Pushed the first (and biggest) commit. More tomorrow.

I hadn't run my buildfarm-compile-warning scraper for a little while,
but I just did, and I find that this commit is causing warnings on
no fewer than 14 buildfarm animals.  They all look like

 ayu           | 2023-02-25 23:02:08 | pgstat_io.c:40:14: warning: compari=
son of constant 2 with expression of type 'IOObject' (aka 'enum IOObject')=
 is always true [-Wtautological-constant-out-of-range-compare]
 ayu           | 2023-02-25 23:02:08 | pgstat_io.c:43:16: warning: compari=
son of constant 4 with expression of type 'IOContext' (aka 'enum IOContext=
') is always true [-Wtautological-constant-out-of-range-compare]
 ayu           | 2023-02-25 23:02:08 | pgstat_io.c:70:19: warning: compari=
son of constant 2 with expression of type 'IOObject' (aka 'enum IOObject')=
 is always true [-Wtautological-constant-out-of-range-compare]
 ayu           | 2023-02-25 23:02:08 | pgstat_io.c:71:20: warning: compari=
son of constant 4 with expression of type 'IOContext' (aka 'enum IOContext=
') is always true [-Wtautological-constant-out-of-range-compare]
 ayu           | 2023-02-25 23:02:08 | pgstat_io.c:115:14: warning: compar=
ison of constant 2 with expression of type 'IOObject' (aka 'enum IOObject'=
) is always true [-Wtautological-constant-out-of-range-compare]
 ayu           | 2023-02-25 23:02:08 | pgstat_io.c:118:16: warning: compar=
ison of constant 4 with expression of type 'IOContext' (aka 'enum IOContex=
t') is always true [-Wtautological-constant-out-of-range-compare]
 ayu           | 2023-02-25 23:02:08 | pgstatfuncs.c:1329:12: warning: com=
parison of constant 2 with expression of type 'IOObject' (aka 'enum IOObje=
ct') is always true [-Wtautological-constant-out-of-range-compare]
 ayu           | 2023-02-25 23:02:08 | pgstatfuncs.c:1334:17: warning: com=
parison of constant 4 with expression of type 'IOContext' (aka 'enum IOCon=
text') is always true [-Wtautological-constant-out-of-range-compare]

That is, these compilers think that comparisons like

	io_object < IOOBJECT_NUM_TYPES
	io_context < IOCONTEXT_NUM_TYPES

are constant-true.  This seems not good; if they were to actually
act on this observation, by removing those loop-ending tests,
we'd have a problem.

The issue seems to be that code like this:

typedef enum IOContext
{
	IOCONTEXT_BULKREAD,
	IOCONTEXT_BULKWRITE,
	IOCONTEXT_NORMAL,
	IOCONTEXT_VACUUM,
} IOContext;

#define IOCONTEXT_FIRST IOCONTEXT_BULKREAD
#define IOCONTEXT_NUM_TYPES (IOCONTEXT_VACUUM + 1)

is far too cute for its own good.  I'm not sure about how to fix it
either.  I thought of defining

#define IOCONTEXT_LAST IOCONTEXT_VACUUM

and make the loop conditions like "io_context <=3D IOCONTEXT_LAST",
but that doesn't actually fix the problem.

(Even aside from that, I do not find this coding even a little bit
mistake-proof: you still have to remember to update the #define
when adding another enum value.)

We have similar code involving enum ForkNumber but it looks to me
like the loop variables are always declared as plain "int".  That
might be the path of least resistance here.

			regards, tom lane