Horton
Menu

Re: Amcheck verification of GiST and GIN

Kirill Reshke <reshkekirill@gmail.com>

View thread
From: Kirill Reshke <reshkekirill@gmail.com>
To: "Andrey M. Borodin" <x4mmm@yandex-team.ru>
Cc: Alexander Lakhin <exclusion@gmail.com>, Andrey Borodin <amborodin86@gmail.com>, Peter Geoghegan <pg@bowt.ie>, Mark Dilger <mark.dilger@enterprisedb.com>, 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>, Tomas Vondra <tomas@vondra.me>
Date: 2024-12-03T15:04:08Z
Lists: pgsql-hackers

Attachments

On Mon, 2 Dec 2024 at 12:57, Kirill Reshke <reshkekirill@gmail.com> wrote:
> This change was not correct at all.
> PFA v32.
>
> Repro:
> ```
> db1=# create table ttt(t text);
> CREATE TABLE
> db1=# create index on ttt using gin(t gin_trgm_ops);
> CREATE INDEX
> db1=# insert into ttt select md5(random()::text) from generate_series(1,50000);
> INSERT 0 50000
> db1=# set client_min_messages to debug5;
> DEBUG:  CommitTransaction(1) name: unnamed; blockState: STARTED;
> state: INPROGRESS, xid/subid/cid: 0/1/0
> SET
> db1=# select gin_index_check('ttt_t_idx');
> DEBUG:  StartTransaction(1) name: unnamed; blockState: DEFAULT; state:
> INPROGRESS, xid/subid/cid: 0/1/0
> DEBUG:  processing entry tree page at blk 1, maxoff: 2
> DEBUG:  processing entry tree page at blk 941, maxoff: 229
> ERROR:  index "ttt_t_idx" has wrong tuple order on entry tree page,
> block 941, offset 229
> db1=#
> ```
> I have only observed failures on the last tuple of the entry page. All
> other known issues that were on v31 are now fixed (I hope).

PFA v33.

Main change from v32 is a meson-related fix, and also bug fix that I
reported in a previous message.

I did find this in README.
```
GIN packs keys and downlinks into tuples in a different way.

(P_0, K_1), (P_1, K_2), ... , (P_n, K_{n+1})

P_i is grouped with K_{i+1}.  -Inf key is not needed.

There are couple of additional notes regarding K_{n+1} key.
1) In entry tree rightmost page, a key coupled with P_n doesn't really matter.
Highkey is assumed to be infinity.
2) In posting tree, a key coupled with P_n always doesn't matter.  Highkey for
non-rightmost pages is stored separately and accessed via
GinDataPageGetRightBound().
```
I indeed only observe gin_index_check failures only on the In entry
tree rightmost (non-leaf & non-root) page, on the last tuple
(Highkey). So fix was not to check is:


```
-                       /* (apparently) first block is metadata, skip
order check */
-                       if (i != FirstOffsetNumber && stack->blkno !=
(BlockNumber) 1)
+                       /*
+                        * First block is metadata, skip order check.
Also, never check
+                        * for high key on rightmost page, as this key
is not really
+                        * stored explicitly.
+                        */
+                       if (i != FirstOffsetNumber && stack->blkno !=
GIN_ROOT_BLKNO &&
+                               !(i == maxoff && rightlink ==
InvalidBlockNumber))
                        {
```

I also did a small wording correction for this part of README (this is v33-0006)

-- 
Best regards,
Kirill Reshke

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.