Horton

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

Peter Geoghegan <pg@bowt.ie>

View thread
From: Peter Geoghegan <pg@bowt.ie>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: Pavel Borisov <pashkin.elfe@gmail.com>, Anastasia Lubennikova <a.lubennikova@postgrespro.ru>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-03-03T02:54:59Z
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 Mon, Mar 1, 2021 at 11:22 AM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
> If bt_index_check() and bt_index_parent_check() are to have this functionality, shouldn't there be an option controlling it much as the option (heapallindexed boolean) controls checking whether all entries in the heap are indexed in the btree?  It seems inconsistent to have an option to avoid checking the heap for that, but not for this.

I agree. Actually, it should probably use the same snapshot as the
heapallindexed=true case. So either only perform unique constraint
verification when that option is used, or invent a new option that
will still share the snapshot used by heapallindexed=true (when the
options are combined).

> The regression test you provided is not portable.  I am getting lots of errors due to differing output of the form "page lsn=0/4DAD7E0".  You might turn this into a TAP test and use a regular expression to check the output.

I would test this using a custom opclass that does simple fault
injection. For example, an opclass that indexes integers, but can be
configured to dynamically make 0 values equal or unequal to each
other. That's more representative of real-world problems.

You "break the warranty" by updating pg_index, even compared to
updating other system catalogs. In particular, you break the
"indcheckxmin wait -- wait for xmin to be old before using index"
stuff in get_relation_info(). So it seems worse than updating
pg_attribute, for example (which is something that the tests do
already).

-- 
Peter Geoghegan