Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index.

Peter Geoghegan <pg@bowt.ie>

From: Peter Geoghegan <pg@bowt.ie>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: Pavel Borisov <pashkin.elfe@gmail.com>, Alexander Korotkov <aekorotkov@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Noah Misch <noah@leadboat.com>, Peter Eisentraut <peter@eisentraut.org>, Aleksander Alekseev <aleksander@timescale.com>, Postgres hackers <pgsql-hackers@lists.postgresql.org>, Maxim Orlov <orlovmg@gmail.com>, Andres Freund <andres@anarazel.de>, Greg Stark <stark@mit.edu>, Julien Rouhaud <rjuju123@gmail.com>, David Steele <david@pgmasters.net>, Maxim Orlov <m.orlov@postgrespro.ru>, lubennikovaav@gmail.com
Date: 2024-05-17T20:00:55Z
Lists: pgsql-hackers

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: Optimize speed of checking for unique constraint violation

  2. amcheck: Report an error when the next page to a leaf is not a leaf

  3. amcheck: Don't load the right sibling page into BtreeCheckState

  4. amcheck: Refactoring the storage of the last visible entry

  5. Teach contrib/amcheck to check the unique constraint violation

  6. Add macros in hash and btree AMs to get the special area of their pages

On Fri, May 17, 2024 at 3:42 PM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
> On further review, the test was not anticipating the error message "high key invariant violated for index".  That wasn't seen in calls to bt_index_parent_check(), but appears as one of the errors from bt_index_check().  I am rerunning the test now....

Many different parts of the B-Tree code will fight against allowing
duplicates of the same value to span multiple leaf pages -- this is
especially true for unique indexes. For example, nbtsplitloc.c has a
variety of strategies that will prevent choosing a split point that
necessitates including a distinguishing heap TID in the new high key.
In other words, nbtsplitloc.c is very aggressive about picking a split
point between (rather than within) groups of duplicates.

Of course it's still *possible* for a unique index to have multiple
leaf pages containing the same individual value. The regression tests
do have coverage for certain relevant code paths (e.g., there is
coverage for code paths only hit when _bt_check_unique has to go to
the page to the right). This is only the case because I went out of my
way to make sure of it, by adding tests that allow a huge number of
version duplicates to accumulate within a unique index. (The "move
right" _bt_check_unique branches had zero test coverage for a year or
two.)

Just how important it is that amcheck covers cases where the version
duplicates span multiple leaf pages is of course debatable -- it's
always better to be more thorough, when practical. But it's certainly
something that needs to be assessed based on the merits.

--
Peter Geoghegan