On Thu, Jul 16, 2026 at 11:25 PM Sofia Kopikova <[email protected]> wrote:
> On 7/15/26 15:00, Stepan Neretin wrote: > > Hi, > > > > gin_index_check() compares adjacent separator keys on posting-tree > > internal pages with ItemPointerCompare(...) < 0, so it reports > > out-of-order keys but misses duplicates. > > > > Equal separators should not occur: each PostingItem key is the child > > page's right bound (dataPrepareDownlink), and leaf TIDs are strictly > > increasing, so siblings must be strictly ascending. dataLocateItem > > also treats equal keys as an exact match to that downlink. Entry-tree > > checks already reject equals (>= 0); posting trees should do the same. > > > > The attached patch changes the comparison to <= 0 and adds a TAP test > > that corrupts two adjacent keys to be equal. > > > > > > Best regards, Stepan Neretin. > > Thanks for your patch, Stepan! > > The fix itself looks good to me, but why don't you make your test in the > same style as other tests? I mean, make 'test' relation name and '2' > block number perl variables, like it's done in the tests above. > > -- > regards, > Sofia Kopikova > Postgres Professional > > Hi Sofia, Thanks for the review! I've updated the TAP test to use $relname / $blkno like the other tests in the file. Attached is v2. Best regards, Stepan Neretin
From 8d86317cf30cdd15044f7834e5141216c5f1bb1e Mon Sep 17 00:00:00 2001 From: snppg <[email protected]> Date: Wed, 15 Jul 2026 18:58:47 +0700 Subject: [PATCH v2] amcheck: Detect duplicate PostingItem keys in GIN posting trees gin_index_check() used a strict < comparison for adjacent separator keys, so equal keys were silently accepted. GIN posting-tree separators must be strictly ascending (each key is a child right bound), matching the entry-tree checks that already reject equals. v2: Style the TAP test like the existing ones ($relname / $blkno). --- contrib/amcheck/t/006_verify_gin.pl | 32 ++++++++++++++++++++++++++++++ contrib/amcheck/verify_gin.c | 2 +- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/contrib/amcheck/t/006_verify_gin.pl b/contrib/amcheck/t/006_verify_gin.pl index 93571f1d2ab..75789b8d848 100644 --- a/contrib/amcheck/t/006_verify_gin.pl +++ b/contrib/amcheck/t/006_verify_gin.pl @@ -37,6 +37,7 @@ 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(); +equal_posting_tree_internal_keys_test(); sub invalid_entry_order_leaf_page_test { @@ -252,6 +253,37 @@ sub inconsistent_with_parent_key__parent_key_corrupted_posting_tree_test like($stderr, qr/$expected/); } +sub equal_posting_tree_internal_keys_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, 30000); + CREATE INDEX $indexname ON $relname USING gin (a); + )); + my $relpath = relation_filepath($indexname); + + $node->stop; + + my $blkno = 2; + + my $find = qr/\A(.{32})(.{4})(.{6})(.{4})(.{6})/s; + my $replace = '$1$2$5$4$5'; + string_replace_block($relpath, $find, $replace, $blkno); + + $node->start; + + my ($result, $stdout, $stderr) = + $node->psql('postgres', qq(SELECT gin_index_check('$indexname'))); + my $expected = + "index \"$indexname\" has wrong tuple order in posting tree, block $blkno, offset 2"; + like($stderr, qr/$expected/); +} + # 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 fa06689ed5b..43b3a164453 100644 --- a/contrib/amcheck/verify_gin.c +++ b/contrib/amcheck/verify_gin.c @@ -334,7 +334,7 @@ gin_check_posting_tree_parent_keys_consistency(Relation rel, BlockNumber posting { PostingItem *previous_posting_item = GinDataPageGetPostingItem(page, i - 1); - if (ItemPointerCompare(&posting_item->key, &previous_posting_item->key) < 0) + if (ItemPointerCompare(&posting_item->key, &previous_posting_item->key) <= 0) ereport(ERROR, (errcode(ERRCODE_INDEX_CORRUPTED), errmsg("index \"%s\" has wrong tuple order in posting tree, block %u, offset %u", -- 2.55.0
