Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.7\))
Subject: Re: BUG #17212: pg_amcheck fails on checking temporary relations
From: Mark Dilger <mark.dilger@enterprisedb.com>
In-Reply-To: 
 <CAH2-Wz=8RP6izkLF=P_bFUK-CyTsO09hQ1-ZsC9P4Qiz3CughQ@mail.gmail.com>
Date: Wed, 6 Oct 2021 10:57:20 -0700
Cc: Pavel Borisov <pashkin.elfe@gmail.com>,
 Alexander Lakhin <exclusion@gmail.com>,
 Robert Haas <robertmhaas@gmail.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <561CE3DB-5CBC-44BF-9A3E-1D7FAEB123E0@enterprisedb.com>
References: <17212-34dd4a1d6bba98bf@postgresql.org>
 <CAH2-Wz=bw-Uk2yoOq7yevC9hyMGgZ2Ud34nd9YJJPXJ3pkVcrw@mail.gmail.com>
 <E85855BD-C063-4B24-9EAA-02DECC8F42B9@enterprisedb.com>
 <E283B45F-5745-49C9-BBC7-99876E2FABD1@enterprisedb.com>
 <f42de8ec-f219-0922-93b3-37a72bba1feb@gmail.com>
 <28B36E41-1E8F-4DC1-A924-711621BA4972@enterprisedb.com>
 <CAH2-Wz=v4py=c79J7wpdVmQMkd8rO7gj5Ur5Dp4E0y7aQUz6cg@mail.gmail.com>
 <386F66EE-DEFD-42EA-9DB2-102BF3414669@enterprisedb.com>
 <CAH2-Wzkwb+1GSONMrXByHhgkzD-N59A_kFcvsoA_+M9cNNHm5g@mail.gmail.com>
 <CA+TgmobJnZE6WMw1y+N9ts36nsbL_un0CYB_khieg8rOa86RBw@mail.gmail.com>
 <CAH2-Wzm02e=jAarpzeZGyK8JyXTvuHCfJ9KC5svosD72nROKrw@mail.gmail.com>
 <B2BDB87C-8288-436E-B3DC-8AEB92FEB8F1@enterprisedb.com>
 <CAH2-Wz=WYdmbc1k8Lr2P4eQeYkh1UYvyDigzrvukejcWc2jKOA@mail.gmail.com>
 <c422a1aa-81c0-0762-0971-9a026cf2aa79@gmail.com>
 <CAH2-WzndLb0-GZcshTJGw6u6SQrJ5puyFiFYh-DWR8ue2zQuvA@mail.gmail.com>
 <CALT9ZEGhuXiH54VgyVJ65V+ccGqPqHTduBM4saPWSD2mrD5C6Q@mail.gmail.com>
 <F824CD5F-C82F-451C-AE67-9AD31447424A@enterprisedb.com>
 <CAH2-WznaYWowxxFaGK6Hv=SUQYS2-r7dQZJFWyDT_W6GGtuCjA@mail.gmail.com>
 <63FF68F5-683D-423E-A667-9C00441101A5@enterprisedb.com>
 <CAH2-Wz=8RP6izkLF=P_bFUK-CyTsO09hQ1-ZsC9P4Qiz3CughQ@mail.gmail.com>
To: Peter Geoghegan <pg@bowt.ie>
Archived-At: 
 <https://www.postgresql.org/message-id/561CE3DB-5CBC-44BF-9A3E-1D7FAEB123E0%40enterprisedb.com>
Precedence: bulk


> On Oct 6, 2021, at 10:39 AM, Peter Geoghegan <pg@bowt.ie> wrote:
>=20
>> The differences in the upcoming version are
>>=20
>> 1) --all no longer means "all relations" but rather "all checkable =
relations"
>=20
> Clearly pg_amcheck never checked all relations, because it never
> checked indexes that are not B-Tree indexes. I'm pretty sure that I
> can poke big holes in almost any positivist statement like that with
> little effort.

There is a distinction here that you are (intentionally?) failing to =
acknowledge.  On the one hand, there are relation types that cannot be =
checked because no checking functions for them exist.  (Hash, gin, gist, =
etc.)  On the other hand, there are relations which could be check but =
for the current state of the system, or could be checked in some =
particular way but for the current state of the system.  One of those =
has to do with code that doesn't exist, and the other has to do with the =
state of the system.  I'm only talking about the second.

>=20
>> 2) checking options should be automatically downgraded under =
circumstances where they cannot be applied
>> 3) unlogged relations during replication are by definition not =
corrupt
>>=20
>> I think #1 and #3 are unsurprising enough that they need no =
documentation update.  #2 is slightly surprising (at least to me) so I =
updated the docs for it.
>=20
> To me #2 sounds like a tautology. It could almost be phrased as
> "pg_amcheck does not check the things that it cannot check".

I totally disagree.  It is uncomfortable to me that `pg_amcheck =
--parent-check` will now silently not perform the parent check that was =
explicitly requested.  That reported an error before, and now it just =
downgrades the check.  This is hardly tautological.  I'm only willing to =
post a patch with that change because I can see a practical argument =
that somebody might run that as a cron job and they don't want the cron =
job failing when the database happens to go into recovery.  But again, =
not at all tautological.

=E2=80=94
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company