Re: Amcheck verification of GiST and GIN

On Mon, 2 Dec 2024 at 12:57, Kirill Reshke <reshkekirill(at)gmail(dot)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