database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-27 12:41:57 +03:00
Code Activity

amcheck: Fix checks of entry order for GIN indexes

Browse Source 
This tightens a couple checks in checking GIN indexes, which might have
resulted in incorrect results (false positives/negatives).

* The code skipped ordering checks if the entries were for different
  attributes (for multi-column GIN indexes), possibly missing some cases
  of data corruption. But the attribute number is part of the ordering,
  so we can check that.

* The root page was skipped when checking entry order, but that is
  unnecessary. The root page is subject to the same ordering rules, we
  can process it just like any other page.

* The high key on the right-most page was not checked, but that is
  needed only for inner pages (we don't store the high key for those).
  For leaf pages we can check the high key just fine.

* Correct the detection of split pages. If the page gets split, the
  cached parent key is greater than the current child key (not less, as
  the code incorrectly expected).

Issues reported by Arseniy Mukhin, along with a proposed patch. Review
by Andrey M. Borodin, cleanup and improvements by me.

Author: Arseniy Mukhin <arseniy.mukhin.dev@gmail.com>
Reviewed-by: Andrey M. Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/CAE7r3MJ611B9TE=YqBBncewp7-k64VWs+sjk7XF6fJUX77uFBA@mail.gmail.com
This commit is contained in:
Tomas Vondra
2025-06-17 14:55:27 +02:00
parent 8dd41c0bff
commit 0b54b39233
3 changed files with 229 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
contrib/amcheck/verify_gin.c
View File

@ -459,17 +459,18 @@ gin_check_parent_keys_consistency(Relation rel,
Datum parent_key = gintuple_get_key(&state,
stack->parenttup,
&parent_key_category);
OffsetNumber parent_key_attnum = gintuple_get_attrnum(&state, stack->parenttup);
ItemId iid = PageGetItemIdCareful(rel, stack->blkno,
page, maxoff);
IndexTuple idxtuple = (IndexTuple) PageGetItem(page, iid);
OffsetNumber attnum = gintuple_get_attrnum(&state, idxtuple);
OffsetNumber page_max_key_attnum = gintuple_get_attrnum(&state, idxtuple);
GinNullCategory page_max_key_category;
Datum page_max_key = gintuple_get_key(&state, idxtuple, &page_max_key_category);
if (rightlink != InvalidBlockNumber &&
ginCompareEntries(&state, attnum, page_max_key,
page_max_key_category, parent_key,
parent_key_category) > 0)
ginCompareAttEntries(&state, page_max_key_attnum, page_max_key,
page_max_key_category, parent_key_attnum,
parent_key, parent_key_category) < 0)
{
/* split page detected, install right link to the stack */
GinScanItem *ptr;
@ -508,9 +509,7 @@ gin_check_parent_keys_consistency(Relation rel,
{
ItemId iid = PageGetItemIdCareful(rel, stack->blkno, page, i);
IndexTuple idxtuple = (IndexTuple) PageGetItem(page, iid);
OffsetNumber attnum = gintuple_get_attrnum(&state, idxtuple);
GinNullCategory prev_key_category;
Datum prev_key;
OffsetNumber current_attnum = gintuple_get_attrnum(&state, idxtuple);
GinNullCategory current_key_category;
Datum current_key;
@ -523,20 +522,24 @@ gin_check_parent_keys_consistency(Relation rel,
current_key = gintuple_get_key(&state, idxtuple, &current_key_category);
/*
* First block is metadata, skip order check. Also, never check
* for high key on rightmost page, as this key is not really
* stored explicitly.
* Compare the entry to the preceding one.
*
* Also make sure to not compare entries for different attnums,
* which may be stored on the same page.
* Don't check for high key on the rightmost inner page, as this
* key is not really stored explicitly.
*
* The entries may be for different attributes, so make sure to
* use ginCompareAttEntries for comparison.
*/
if (i != FirstOffsetNumber && attnum == prev_attnum && stack->blkno != GIN_ROOT_BLKNO &&
!(i == maxoff && rightlink == InvalidBlockNumber))
if ((i != FirstOffsetNumber) &&
!(i == maxoff && rightlink == InvalidBlockNumber && !GinPageIsLeaf(page)))
{
Datum prev_key;
GinNullCategory prev_key_category;
prev_key = gintuple_get_key(&state, prev_tuple, &prev_key_category);
if (ginCompareEntries(&state, attnum, prev_key,
prev_key_category, current_key,
current_key_category) >= 0)
if (ginCompareAttEntries(&state, prev_attnum, prev_key,
prev_key_category, current_attnum,
current_key, current_key_category) >= 0)
ereport(ERROR,
(errcode(ERRCODE_INDEX_CORRUPTED),
errmsg("index \"%s\" has wrong tuple order on entry tree page, block %u, offset %u, rightlink %u",
@ -551,13 +554,14 @@ gin_check_parent_keys_consistency(Relation rel,
i == maxoff)
{
GinNullCategory parent_key_category;
OffsetNumber parent_key_attnum = gintuple_get_attrnum(&state, stack->parenttup);
Datum parent_key = gintuple_get_key(&state,
stack->parenttup,
&parent_key_category);
if (ginCompareEntries(&state, attnum, current_key,
current_key_category, parent_key,
parent_key_category) > 0)
if (ginCompareAttEntries(&state, current_attnum, current_key,
current_key_category, parent_key_attnum,
parent_key, parent_key_category) > 0)
{
/*
* There was a discrepancy between parent and child
@ -576,6 +580,7 @@ gin_check_parent_keys_consistency(Relation rel,
stack->blkno, stack->parentblk);
else
{
parent_key_attnum = gintuple_get_attrnum(&state, stack->parenttup);
parent_key = gintuple_get_key(&state,
stack->parenttup,
&parent_key_category);
@ -584,9 +589,9 @@ gin_check_parent_keys_consistency(Relation rel,
* Check if it is properly adjusted. If succeed,
* proceed to the next key.
*/
if (ginCompareEntries(&state, attnum, current_key,
current_key_category, parent_key,
parent_key_category) > 0)
if (ginCompareAttEntries(&state, current_attnum, current_key,
current_key_category, parent_key_attnum,
parent_key, parent_key_category) > 0)
ereport(ERROR,
(errcode(ERRCODE_INDEX_CORRUPTED),
errmsg("index \"%s\" has inconsistent records on page %u offset %u",
@ -638,7 +643,7 @@ gin_check_parent_keys_consistency(Relation rel,
}
prev_tuple = CopyIndexTuple(idxtuple);
prev_attnum = attnum;
prev_attnum = current_attnum;
}
LockBuffer(buffer, GIN_UNLOCK);