v5-0004-patch-3-gin_check_posting_tree_parent_keys_consis.patch

From 3f19acdaaeaa7db51ef66adf58befc5598339683 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@vondra.me>
Date: Mon, 9 Jun 2025 01:46:13 +0200
Subject: [PATCH v5 4/4] patch 3:
 gin_check_posting_tree_parent_keys_consistency

6) In posting tree parent key check part:

              /*
              * Check if this tuple is consistent with the downlink in the
              * parent.
              */
             if (stack->parentblk != InvalidBlockNumber && i == maxoff &&
                ItemPointerCompare(&stack->parentkey, &posting_item->key) < 0)
                ereport(ERROR,
                      ...
Here we don't check if stack->parentkey is valid, so sometimes we
compare invalid parentkey (because we can have
valid parentblk and invalid parentkey the same time). Invalid
parentkey is always bigger, so the code never triggers
ereport, but it doesn't look right. so probably we can rewrite it this way:

                if (i == maxoff && ItemPointerIsValid(&stack->parentkey) &&
                ItemPointerCompare(&stack->parentkey, &posting_item->key) < 0)

7) When producing stack entries for posting tree check, we set parent
key like this:

             /*
              * Set rightmost parent key to invalid item pointer. Its value
              * is 'Infinity' and not explicitly stored.
              */
             if (rightlink == InvalidBlockNumber)
                ItemPointerSetInvalid(&ptr->parentkey);
             else
                ptr->parentkey = posting_item->key;

We set invalid parent key for all items of the rightmost page. But
it's the only rightmost item that doesn't have an explicit
parentkey (actually the comment says exactly this, but the code does a
different thing). All others have an explicit parent
key and we can set it. So fix can look like this:

                if (rightlink == InvalidBlockNumber && i == maxoff)
                ItemPointerSetInvalid(&ptr->parentkey);
             else
                ptr->parentkey = posting_item->key;

But for (rightlink == InvalidBlockNumber && i == maxoff)
posting_item->key is always (0,0) (we check it a little bit earlier),
so I think we can simplify it:

                ptr->parentkey = posting_item->key;
---
 contrib/amcheck/t/006_verify_gin.pl | 39 +++++++++++++++++++++++++++++
 contrib/amcheck/verify_gin.c        | 12 +++------
 2 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/contrib/amcheck/t/006_verify_gin.pl b/contrib/amcheck/t/006_verify_gin.pl
index a999a13d183..e8afeb038fc 100644
--- a/contrib/amcheck/t/006_verify_gin.pl
+++ b/contrib/amcheck/t/006_verify_gin.pl
@@ -33,6 +33,7 @@ invalid_entry_order_inner_page_test();
 invalid_entry_columns_order_test();
 inconsistent_with_parent_key__parent_key_corrupted_test();
 inconsistent_with_parent_key__child_key_corrupted_test();
+inconsistent_with_parent_key__parent_key_corrupted_posting_tree_test();
 
 sub invalid_entry_order_leaf_page_test
 {
@@ -236,6 +237,44 @@ sub inconsistent_with_parent_key__child_key_corrupted_test
 	ok($stderr =~ "index \"$indexname\" has inconsistent records on page 5 offset 3");
 }
 
+sub inconsistent_with_parent_key__parent_key_corrupted_posting_tree_test
+{
+	my $relname = "test";
+	my $indexname = "test_gin_idx";
+
+	$node->safe_psql(
+		'postgres', qq(
+		DROP TABLE IF EXISTS $relname;
+		CREATE TABLE $relname (a text[]);
+		INSERT INTO $relname (a) select ('{aaaaa}') from generate_series(1,10000);
+		CREATE INDEX $indexname ON $relname USING gin (a);
+	));
+	my $relpath = relation_filepath($indexname);
+
+	$node->stop;
+
+	my $blkno = 2;  # posting tree root
+
+	# we have a posting tree for 'aaaaa' key with the root at 2nd block
+	# and two leaf pages 3 and 4. for 4th page high key is (65,52), so let's make it a little bit
+	# smaller, so that there are tid's in leaf page that are larger then the new high key.
+	my $find = pack('S', 65) . pack('S', 52);
+	my $replace = '"' . pack('S', 64) . pack('S', 52) . '"';
+	string_replace_block(
+		$relpath,
+		$find,
+		$replace,
+		$blksize,
+		$blkno
+	);
+
+	$node->start;
+
+	my ($result, $stdout, $stderr) = $node->psql('postgres', qq(SELECT gin_index_check('$indexname')));
+	ok($stderr =~ "index \"$indexname\": tid exceeds parent's high key in postingTree leaf on block 4");
+}
+
+
 # Returns the filesystem path for the named relation.
 sub relation_filepath
 {
diff --git a/contrib/amcheck/verify_gin.c b/contrib/amcheck/verify_gin.c
index 8f6a5410cb7..427cf1669a6 100644
--- a/contrib/amcheck/verify_gin.c
+++ b/contrib/amcheck/verify_gin.c
@@ -346,7 +346,7 @@ gin_check_posting_tree_parent_keys_consistency(Relation rel, BlockNumber posting
 				 * Check if this tuple is consistent with the downlink in the
 				 * parent.
 				 */
-				if (stack->parentblk != InvalidBlockNumber && i == maxoff &&
+				if (i == maxoff && ItemPointerIsValid(&stack->parentkey) &&
 					ItemPointerCompare(&stack->parentkey, &posting_item->key) < 0)
 					ereport(ERROR,
 							(errcode(ERRCODE_INDEX_CORRUPTED),
@@ -359,14 +359,10 @@ gin_check_posting_tree_parent_keys_consistency(Relation rel, BlockNumber posting
 				ptr->depth = stack->depth + 1;
 
 				/*
-				 * Set rightmost parent key to invalid item pointer. Its value
-				 * is 'Infinity' and not explicitly stored.
+				 * The rightmost parent key is always invalid item pointer.
+				 * Its value is 'Infinity' and not explicitly stored.
 				 */
-				if (rightlink == InvalidBlockNumber)
-					ItemPointerSetInvalid(&ptr->parentkey);
-				else
-					ptr->parentkey = posting_item->key;
-
+				ptr->parentkey = posting_item->key;
 				ptr->parentblk = stack->blkno;
 				ptr->blkno = BlockIdGetBlockNumber(&posting_item->child_blkno);
 				ptr->next = stack->next;
-- 
2.49.0