Re: Amcheck verification of GiST and GIN

Tomas Vondra <tomas@vondra.me>

From: Tomas Vondra <tomas@vondra.me>
To: Arseniy Mukhin <arseniy.mukhin.dev@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Kirill Reshke <reshkekirill@gmail.com>, Mark Dilger <mark.dilger@enterprisedb.com>, "Andrey M. Borodin" <x4mmm@yandex-team.ru>, Alexander Lakhin <exclusion@gmail.com>, Andrey Borodin <amborodin86@gmail.com>, Peter Geoghegan <pg@bowt.ie>, Jose Arthur Benetasso Villanova <jose.arthur@gmail.com>, Andres Freund <andres@anarazel.de>, Nikolay Samokhvalov <samokhvalov@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-05-26T10:27:45Z
Lists: pgsql-hackers

Attachments

Hello Arseniy,

I finally got time to look at this more closely, and do some testing.

Are there any cases when the current code incorrectly reports corruption
for a valid index? So far I've been unable to find such case. Or am I wrong?

It seems to me all the proposed changes are "tightening" the checks, in
the sense that we might have missed certain types of issues before. This
is supported by the fact that the new TAP test fails on master, i.e.
master does not report the corruption the TAP introduces.

(The TAP test is great, it would have been great to add something like
this in the original commit.)

Also, I've noticed that the TAP test passes even with some (most) of the
verify_gin.c changes reverted. See the 0002 patch - this does not break
the TAP test. Of course, that does not prove the changes are wrong and
I'm not claiming that. But can we improve the TAP test to trigger this
too? To show the current code (in master) misses this?

Grigory, Andrey, Heikki, any opinions on the tweaks?


regards

-- 
Tomas Vondra

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. amcheck: Fix posting tree checks in gin_index_check()

  2. amcheck: Fix parent key check in gin_index_check()

  3. amcheck: Fix checks of entry order for GIN indexes

  4. amcheck: Remove unused GinScanItem->parentlsn field

  5. amcheck: Test gin_index_check on a multicolumn index

  6. Remove incidental md5() function use from test

  7. amcheck: Add a GIN index to the CREATE INDEX CONCURRENTLY tests

  8. amcheck: Add a test with GIN index on JSONB data

  9. amcheck: Fix indentation in verify_gin.c

  10. amcheck: Add gin_index_check() to verify GIN index

  11. amcheck: Move common routines into a separate module

  12. Fix grammar in GIN README

  13. Avoid amcheck inline compression false positives.