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-Wzm0k++pWW4T8SyKWHn1uQd17Ay5=ooZZPZoZE_fS7i49g@mail.gmail.com>
Date: Mon, 11 Oct 2021 19:22:14 -0700
Cc: Robert Haas <robertmhaas@gmail.com>,
 Pavel Borisov <pashkin.elfe@gmail.com>,
 Alexander Lakhin <exclusion@gmail.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <C26F0D76-7C5E-43FD-810C-32459B1104EC@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>
 <561CE3DB-5CBC-44BF-9A3E-1D7FAEB123E0@enterprisedb.com>
 <CA+TgmobtAq9sNa0Z4QNx0xiP4NAobuGOnOirTWdJajkTRFvXNw@mail.gmail.com>
 <CAH2-WzmJkVKFSqJd2_uy07jPg-LW0yCHh0fgA8rFkdyk6FZOnQ@mail.gmail.com>
 <CA+TgmoYa5exExaONEnGBWRdi7A3=FKuKzewY8DXTj6+045wiFQ@mail.gmail.com>
 <CAH2-WzkwRUayddmb3uxCadV_y--DHfY+UMmjtOokiAO2xNh5fA@mail.gmail.com>
 <CA+Tgmoat1M4MAqUQM1H+bkWR=wXmi33bB0YRR=xaFk9ADs5Jeg@mail.gmail.com>
 <CAH2-Wzk3NzXYXsUKWBFuBm6aiXTh-_L2Dm9wPqfFenUvG+VbRw@mail.gmail.com>
 <543F3BF3-EDED-4C52-AF05-623921B6C409@enterprisedb.com>
 <CAH2-WzkXphRHgbaUXqMgb7SBmc_VbwdyWGYqphV6KbOH7stfxA@mail.gmail.com>
 <55A2BFE7-AAAF-4A64-BD3A-5C98EA982267@enterprisedb.com>
 <CAH2-WzniogVqNuWzNVLOKjkd8FBcSry29hN9=wiROUPw92fmGg@mail.gmail.com>
 <07159C9A-40DA-47BF-B2F4-203F5AC00890@enterprisedb.com>
 <CAH2-Wzkt-umhXfR1nqy=ZDXMfoN3LDK4W5VtchfWyRAuPWxtnA@mail.gmail.com>
 <40DB088B-C96F-49B2-BC84-BDC8D35C7E97@enterprisedb.com>
 <CAH2-Wzne7L9fX2LdZ9enNZpO2wuqbrsubCw2thH__CC_nZazjw@mail.gmail.com>
 <0B7DA7A4-2872-485B-BFDD-BF8E9BB07B26@enterprisedb.com>
 <CAH2-Wz=YnyANaKQDfx9pg0YNPS-LY66CXQrUzED4RgmmdS7utA@mail.gmail.com>
 <CAH2-Wzk_pukOFY7JmdiFLsrz+Pd3V8OwgC1TH2Vd5BH5ZgK4bA@mail.gmail.com>
 <AB2D6C6B-9073-45AB-9574-A6E37B855CF3@enterprisedb.com>
 <44A67B65-28BF-4291-A0F0-806DD4309B18@enterprisedb.com>
 <CAH2-WznCQU7D0aNto5r8V6Qy7JOn+Saaev6jzVrY6GBZLmLFSA@mail.gmail.com>
 <F966FBB2-7706-483F-92AE-A99F55BD3926@enterprisedb.com>
 <CAH2-Wzm0k++pWW4T8SyKWHn1uQd17Ay5=ooZZPZoZE_fS7i49g@mail.gmail.com>
To: Peter Geoghegan <pg@bowt.ie>
Archived-At: 
 <https://www.postgresql.org/message-id/C26F0D76-7C5E-43FD-810C-32459B1104EC%40enterprisedb.com>
Precedence: bulk


> On Oct 11, 2021, at 5:37 PM, Peter Geoghegan <pg@bowt.ie> wrote:
>=20
> Cool. I pushed just the amcheck changes a moment ago. I attach the
> remaining changes from your v3, with a new draft commit message (no
> real changes). I didn't push the rest (what remains in the attached
> revision) just yet because I'm not quite sure about the approach used
> to exclude temp tables.

Thanks for that.

> Do we really need the redundancy between prepare_btree_command(),
> prepare_heap_command(), and compile_relation_list_one_db()? All three
> exclude temp relations, plus you have extra stuff in
> prepare_btree_command(). There is some theoretical value in delaying
> the index specific stuff until the query actually runs, at least in
> theory. But it also seems unlikely to make any appreciable difference
> to the overall level of coverage in practice.

I agree that it is unlikely to make much difference in practice.  =
Another session running reindex concurrently is, I think, the most =
likely to conflict, but it is just barely imaginable that a relation =
will be dropped, and its OID reused for something unrelated, by the time =
the check command gets run.  The new object might be temporary where the =
old object was not.  On a properly functioning database, that may be too =
remote a possibility to be worth worrying about, but on a corrupt =
database, most bets are off, and I can't really tell you if that's a =
likely scenario, because it is hard to think about all the different =
ways corruption might cause a database to behave.  On the other hand, =
the join against pg_class might fail due to unspecified corruption, so =
my attempt to play it safe may backfire.

I don't feel strongly about this.  If you'd like me to remove those =
checks, I can do so.  These are just my thoughts on the subject.

> Would it be simpler to do it all together, in
> compile_relation_list_one_db()? Were you concerned about things
> changing when parallel workers are run? Or something else?

Yeah, I was contemplating things changing by the time the parallel =
workers run the command.  I don't know how important that is.

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