On 5/29/25 13:53, Arseniy Mukhin wrote:
> On Mon, May 26, 2025 at 7:28 PM Arseniy Mukhin
> <arseniy.mukhin....@gmail.com> wrote:
>> On Mon, May 26, 2025 at 1:27 PM Tomas Vondra <to...@vondra.me> wrote:
>>> Also, I've noticed that the TAP test passes even with some (most) of the
>>> verify_gin.c changes reverted. See the 0002 patch - this does not break
>>> the TAP test. Of course, that does not prove the changes are wrong and
>>> I'm not claiming that. But can we improve the TAP test to trigger this
>>> too? To show the current code (in master) misses this?
>>
>> Yes, changes in the undo patch is about posting tree check part (6, 7 points)
>> and I haven't written tests for it, because to break posting tree you need to
>> manipulate with tids which is not as easy as replace "aaaa" with "cccc" as
>> tests
>> for entry tree do. Probably it would be much easier to use page api to
>> corrupt some
>> posting tree pages, but I don't know, is it impossible in TAP tests?
>
> I added the test for the posting tree parent_key check. Now applying
> 'undo patch' results in a test failure.
Great, thank you.
I noticed git-am complaining about a couple whitespace issues in the
test, mostly about mixing spaces/tabs. The v4 fixes them (in a separate
part, but should be merged into 0001). It's a detail, but might be good
to try git-am on patches ;-)
> Also I realized that the test 'invalid_entry_columns_order_test' will
> fail on big endian machines,
> because varlena len encoding is different for little endian and big
> endian, so I changed the test a little bit.
> Now the test doesn't use varlena len byte in regex.
I think it'd make sense to split this into smaller patches, each fixing
a different issue. Not one patch for each of the 11 items in your
original message, that would be an overkill ...
I propose to split it like this, into three parts, each addressing a
particular type of mistake:
1) gin_check_posting_tree_parent_keys_consistency
2) gin_check_parent_keys_consistency / att comparisons
3) gin_check_parent_keys_consistency / setting ptr->parenttup (at the end)
Does this make sense to you? If yes, can you split the patch series like
this, including a commit message for each part, explaining the fix? We'd
need the commit message even with a single patch, ofc.
> I also remove the blksize hardcode and start getting it from the
> cluster configuration. But anyway some tests
> will fail with not standard block size (probably all tests where tree
> growth is expected).
>
I think that's fine. AFAIK we don't expect tests to be 100% stable with
other block sizes. It shouldn't crash / segfault, ofc, but some tests
may be sensitive to this.
BTW I hoped to get this fix pushed this week, but that didn't happen and
I'll be away most of next week :-( Let's try to get this sorted so that
I can push it on June 16 or so.
regards
--
Tomas Vondra
From f24396e4cbfa5d5b64cb815654a50bbd9d003154 Mon Sep 17 00:00:00 2001
From: Arseniy Mukhin <arseniy.mukhin....@gmail.com>
Date: Thu, 29 May 2025 13:47:47 +0300
Subject: [PATCH v4 1/3] verify-gin-fixes-and-tests
---
contrib/amcheck/expected/check_gin.out | 12 +
contrib/amcheck/meson.build | 1 +
contrib/amcheck/sql/check_gin.sql | 10 +
contrib/amcheck/t/006_verify_gin.pl | 313 +++++++++++++++++++++++++
contrib/amcheck/verify_gin.c | 57 +++--
5 files changed, 363 insertions(+), 30 deletions(-)
create mode 100644 contrib/amcheck/t/006_verify_gin.pl
diff --git a/contrib/amcheck/expected/check_gin.out b/contrib/amcheck/expected/check_gin.out
index b4f0b110747..8dd01ced8d1 100644
--- a/contrib/amcheck/expected/check_gin.out
+++ b/contrib/amcheck/expected/check_gin.out
@@ -76,3 +76,15 @@ SELECT gin_index_check('gin_check_jsonb_idx');
-- cleanup
DROP TABLE gin_check_jsonb;
+-- Test GIN multicolumn index
+CREATE TABLE "gin_check_multicolumn"(a text[], b text[]);
+INSERT INTO gin_check_multicolumn (a,b) values ('{a,c,e}','{b,d,f}');
+CREATE INDEX "gin_check_multicolumn_idx" on gin_check_multicolumn USING GIN(a,b);
+SELECT gin_index_check('gin_check_multicolumn_idx');
+ gin_index_check
+-----------------
+
+(1 row)
+
+-- cleanup
+DROP TABLE gin_check_multicolumn;
diff --git a/contrib/amcheck/meson.build b/contrib/amcheck/meson.build
index b33e8c9b062..1f0c347ed54 100644
--- a/contrib/amcheck/meson.build
+++ b/contrib/amcheck/meson.build
@@ -49,6 +49,7 @@ tests += {
't/003_cic_2pc.pl',
't/004_verify_nbtree_unique.pl',
't/005_pitr.pl',
+ 't/006_verify_gin.pl',
],
},
}
diff --git a/contrib/amcheck/sql/check_gin.sql b/contrib/amcheck/sql/check_gin.sql
index 66f42c34311..11caed3d6a8 100644
--- a/contrib/amcheck/sql/check_gin.sql
+++ b/contrib/amcheck/sql/check_gin.sql
@@ -50,3 +50,13 @@ SELECT gin_index_check('gin_check_jsonb_idx');
-- cleanup
DROP TABLE gin_check_jsonb;
+
+-- Test GIN multicolumn index
+CREATE TABLE "gin_check_multicolumn"(a text[], b text[]);
+INSERT INTO gin_check_multicolumn (a,b) values ('{a,c,e}','{b,d,f}');
+CREATE INDEX "gin_check_multicolumn_idx" on gin_check_multicolumn USING GIN(a,b);
+
+SELECT gin_index_check('gin_check_multicolumn_idx');
+
+-- cleanup
+DROP TABLE gin_check_multicolumn;
diff --git a/contrib/amcheck/t/006_verify_gin.pl b/contrib/amcheck/t/006_verify_gin.pl
new file mode 100644
index 00000000000..5d974228644
--- /dev/null
+++ b/contrib/amcheck/t/006_verify_gin.pl
@@ -0,0 +1,313 @@
+
+# Copyright (c) 2021-2025, PostgreSQL Global Development Group
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+
+use Test::More;
+
+my $node;
+my $blksize;
+
+#
+# Test set-up
+#
+$node = PostgreSQL::Test::Cluster->new('test');
+$node->init(no_data_checksums => 1);
+$node->append_conf('postgresql.conf', 'autovacuum=off');
+$node->start;
+$blksize = int($node->safe_psql('postgres', 'SHOW block_size;'));
+$node->safe_psql('postgres', q(CREATE EXTENSION amcheck));
+$node->safe_psql(
+ 'postgres', q(
+ CREATE OR REPLACE FUNCTION random_string( INT ) RETURNS text AS $$
+ SELECT string_agg(substring('0123456789bcdfghjkmnpqrstvwxyz', ceil(random() * 30)::integer, 1), '') from generate_series(1, $1);
+ $$ LANGUAGE SQL;));
+
+# Tests
+invalid_entry_order_leaf_page_test();
+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
+{
+ my $relname = "test";
+ my $indexname = "test_gin_idx";
+
+ $node->safe_psql(
+ 'postgres', qq(
+ DROP TABLE IF EXISTS $relname;
+ CREATE TABLE $relname (a text[]);
+ CREATE INDEX $indexname ON $relname USING gin (a);
+ INSERT INTO $relname (a) VALUES ('{aaaaa,bbbbb}');
+ SELECT gin_clean_pending_list('$indexname');
+ ));
+ my $relpath = relation_filepath($indexname);
+
+ $node->stop;
+
+ my $blkno = 1; # root
+
+ # produce wrong order by replacing aaaaa with ccccc
+ string_replace_block(
+ $relpath,
+ "aaaaa",
+ '"ccccc"',
+ $blksize,
+ $blkno
+ );
+
+ $node->start;
+
+ my ($result, $stdout, $stderr) = $node->psql('postgres', qq(SELECT gin_index_check('$indexname')));
+ ok($stderr =~ "index \"$indexname\" has wrong tuple order on entry tree page, block 1, offset 2, rightlink 4294967295");
+}
+
+sub invalid_entry_order_inner_page_test
+{
+ my $relname = "test";
+ my $indexname = "test_gin_idx";
+
+ $node->safe_psql(
+ 'postgres', qq(
+ DROP TABLE IF EXISTS $relname;
+ CREATE TABLE $relname (a text[]);
+ CREATE INDEX $indexname ON $relname USING gin (a);
+ INSERT INTO $relname (a) VALUES (('{' || 'pppppppppp' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'qqqqqqqqqq' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'rrrrrrrrrr' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'ssssssssss' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'tttttttttt' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'uuuuuuuuuu' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'vvvvvvvvvv' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'wwwwwwwwww' || random_string(1870) ||'}')::text[]);
+ SELECT gin_clean_pending_list('$indexname');
+ ));
+ my $relpath = relation_filepath($indexname);
+
+ $node->stop;
+
+ my $blkno = 1; # root
+
+ # we have rrrrrrrrr... and tttttttttt... as keys in the root, so produce wrong order by replacing rrrrrrrrrr....
+ string_replace_block(
+ $relpath,
+ "rrrrrrrrrr",
+ '"zzzzzzzzzz"',
+ $blksize,
+ $blkno
+ );
+
+ $node->start;
+
+ my ($result, $stdout, $stderr) = $node->psql('postgres', qq(SELECT gin_index_check('$indexname')));
+ ok($stderr =~ "index \"$indexname\" has wrong tuple order on entry tree page, block 1, offset 2, rightlink 4294967295");
+}
+
+sub invalid_entry_columns_order_test
+{
+ my $relname = "test";
+ my $indexname = "test_gin_idx";
+
+ $node->safe_psql(
+ 'postgres', qq(
+ DROP TABLE IF EXISTS $relname;
+ CREATE TABLE $relname (a text[],b text[]);
+ CREATE INDEX $indexname ON $relname USING gin (a,b);
+ INSERT INTO $relname (a,b) VALUES ('{aaa}','{bbb}');
+ SELECT gin_clean_pending_list('$indexname');
+ ));
+ my $relpath = relation_filepath($indexname);
+
+ $node->stop;
+
+ my $blkno = 1; # root
+
+ # mess column numbers
+ # root items order before: (1,aaa), (2,bbb)
+ # root items order after: (2,aaa), (1,bbb)
+ my $attrno_1 = pack('s', 1);
+ my $attrno_2 = pack('s', 2);
+
+ my $find = qr/($attrno_1)(.)(aaa)/s;
+ my $replace = '"' . $attrno_2 . '$2$3"';
+ string_replace_block(
+ $relpath,
+ $find,
+ $replace,
+ $blksize,
+ $blkno
+ );
+
+ $find = qr/($attrno_2)(.)(bbb)/s;
+ $replace = '"' . $attrno_1 . '$2$3"';
+ 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\" has wrong tuple order on entry tree page, block 1, offset 2, rightlink 4294967295");
+}
+
+sub inconsistent_with_parent_key__parent_key_corrupted_test
+{
+ my $relname = "test";
+ my $indexname = "test_gin_idx";
+
+ $node->safe_psql(
+ 'postgres', qq(
+ DROP TABLE IF EXISTS $relname;
+ CREATE TABLE $relname (a text[]);
+ CREATE INDEX $indexname ON $relname USING gin (a);
+ INSERT INTO $relname (a) VALUES (('{' || 'llllllllll' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'mmmmmmmmmm' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'nnnnnnnnnn' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'xxxxxxxxxx' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'yyyyyyyyyy' || random_string(1870) ||'}')::text[]);
+ SELECT gin_clean_pending_list('$indexname');
+ ));
+ my $relpath = relation_filepath($indexname);
+
+ $node->stop;
+
+ my $blkno = 1; # root
+
+ # we have nnnnnnnnnn... as parent key in the root, so replace it with something smaller then child's keys
+ string_replace_block(
+ $relpath,
+ "nnnnnnnnnn",
+ '"aaaaaaaaaa"',
+ $blksize,
+ $blkno
+ );
+
+ $node->start;
+
+ my ($result, $stdout, $stderr) = $node->psql('postgres', qq(SELECT gin_index_check('$indexname')));
+ ok($stderr =~ "index \"$indexname\" has inconsistent records on page 5 offset 3");
+}
+
+sub inconsistent_with_parent_key__child_key_corrupted_test
+{
+ my $relname = "test";
+ my $indexname = "test_gin_idx";
+
+ $node->safe_psql(
+ 'postgres', qq(
+ DROP TABLE IF EXISTS $relname;
+ CREATE TABLE $relname (a text[]);
+ CREATE INDEX $indexname ON $relname USING gin (a);
+ INSERT INTO $relname (a) VALUES (('{' || 'llllllllll' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'mmmmmmmmmm' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'nnnnnnnnnn' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'xxxxxxxxxx' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'yyyyyyyyyy' || random_string(1870) ||'}')::text[]);
+ SELECT gin_clean_pending_list('$indexname');
+ ));
+ my $relpath = relation_filepath($indexname);
+
+ $node->stop;
+
+ my $blkno = 5; # leaf
+
+ # we have nnnnnnnnnn... as parent key in the root, so replace child key with something bigger
+ string_replace_block(
+ $relpath,
+ "nnnnnnnnnn",
+ '"pppppppppp"',
+ $blksize,
+ $blkno
+ );
+
+ $node->start;
+
+ my ($result, $stdout, $stderr) = $node->psql('postgres', qq(SELECT gin_index_check('$indexname')));
+ 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
+{
+ my ($relname) = @_;
+
+ my $pgdata = $node->data_dir;
+ my $rel = $node->safe_psql('postgres',
+ qq(SELECT pg_relation_filepath('$relname')));
+ die "path not found for relation $relname" unless defined $rel;
+ return "$pgdata/$rel";
+}
+
+sub string_replace_block {
+ my ($filename, $find, $replace, $blksize, $blkno) = @_;
+
+ my $fh;
+ open($fh, '+<', $filename) or BAIL_OUT("open failed: $!");
+ binmode $fh;
+
+ my $offset = $blkno * $blksize;
+ my $buffer;
+
+ sysseek($fh, $offset, 0) or BAIL_OUT("seek failed: $!");
+ sysread($fh, $buffer, $blksize) or BAIL_OUT("read failed: $!");
+
+ $buffer =~ s/$find/$replace/gee;
+
+ sysseek($fh, $offset, 0) or BAIL_OUT("seek failed: $!");
+ syswrite($fh, $buffer) or BAIL_OUT("write failed: $!");
+
+ close($fh) or BAIL_OUT("close failed: $!");
+
+ return;
+}
+
+done_testing();
\ No newline at end of file
diff --git a/contrib/amcheck/verify_gin.c b/contrib/amcheck/verify_gin.c
index b5f363562e3..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;
@@ -463,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;
@@ -528,20 +525,18 @@ gin_check_parent_keys_consistency(Relation rel,
current_key = gintuple_get_key(&state, idxtuple, ¤t_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.
+ * Never check for high key on rightmost inner page, as this key
+ * is not really stored explicitly.
*
* Also make sure to not compare entries for different attnums,
* which may be stored on the same page.
*/
- if (i != FirstOffsetNumber && attnum == prev_attnum && stack->blkno != GIN_ROOT_BLKNO &&
- !(i == maxoff && rightlink == InvalidBlockNumber))
+ if (i != FirstOffsetNumber && !(i == maxoff && rightlink == InvalidBlockNumber && !GinPageIsLeaf(page)))
{
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, 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",
@@ -556,13 +551,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, attnum, current_key,
+ current_key_category, parent_key_attnum,
+ parent_key, parent_key_category) > 0)
{
/*
* There was a discrepancy between parent and child
@@ -581,6 +577,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);
@@ -589,9 +586,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, 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",
@@ -608,10 +605,10 @@ gin_check_parent_keys_consistency(Relation rel,
ptr = (GinScanItem *) palloc(sizeof(GinScanItem));
ptr->depth = stack->depth + 1;
/* last tuple in layer has no high key */
- if (i != maxoff && !GinPageGetOpaque(page)->rightlink)
- ptr->parenttup = CopyIndexTuple(idxtuple);
- else
+ if (i == maxoff && rightlink == InvalidBlockNumber)
ptr->parenttup = NULL;
+ else
+ ptr->parenttup = CopyIndexTuple(idxtuple);
ptr->parentblk = stack->blkno;
ptr->blkno = GinGetDownlink(idxtuple);
ptr->parentlsn = lsn;
@@ -749,7 +746,7 @@ gin_refind_parent(Relation rel, BlockNumber parentblkno,
ItemId p_iid = PageGetItemIdCareful(rel, parentblkno, parentpage, o);
IndexTuple itup = (IndexTuple) PageGetItem(parentpage, p_iid);
- if (ItemPointerGetBlockNumber(&(itup->t_tid)) == childblkno)
+ if (GinGetDownlink(itup) == childblkno)
{
/* Found it! Make copy and return it */
result = CopyIndexTuple(itup);
--
2.49.0
From accb47ff16f06de2dc45524a1cf1223cefb93272 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <to...@vondra.me>
Date: Sun, 8 Jun 2025 22:29:31 +0200
Subject: [PATCH v4 2/3] whitespace fixes
---
contrib/amcheck/t/006_verify_gin.pl | 69 +++++++++++++++--------------
1 file changed, 35 insertions(+), 34 deletions(-)
diff --git a/contrib/amcheck/t/006_verify_gin.pl b/contrib/amcheck/t/006_verify_gin.pl
index 5d974228644..e8afeb038fc 100644
--- a/contrib/amcheck/t/006_verify_gin.pl
+++ b/contrib/amcheck/t/006_verify_gin.pl
@@ -23,9 +23,9 @@ $blksize = int($node->safe_psql('postgres', 'SHOW block_size;'));
$node->safe_psql('postgres', q(CREATE EXTENSION amcheck));
$node->safe_psql(
'postgres', q(
- CREATE OR REPLACE FUNCTION random_string( INT ) RETURNS text AS $$
- SELECT string_agg(substring('0123456789bcdfghjkmnpqrstvwxyz', ceil(random() * 30)::integer, 1), '') from generate_series(1, $1);
- $$ LANGUAGE SQL;));
+ CREATE OR REPLACE FUNCTION random_string( INT ) RETURNS text AS $$
+ SELECT string_agg(substring('0123456789bcdfghjkmnpqrstvwxyz', ceil(random() * 30)::integer, 1), '') from generate_series(1, $1);
+ $$ LANGUAGE SQL;));
# Tests
invalid_entry_order_leaf_page_test();
@@ -43,8 +43,8 @@ sub invalid_entry_order_leaf_page_test
$node->safe_psql(
'postgres', qq(
DROP TABLE IF EXISTS $relname;
- CREATE TABLE $relname (a text[]);
- CREATE INDEX $indexname ON $relname USING gin (a);
+ CREATE TABLE $relname (a text[]);
+ CREATE INDEX $indexname ON $relname USING gin (a);
INSERT INTO $relname (a) VALUES ('{aaaaa,bbbbb}');
SELECT gin_clean_pending_list('$indexname');
));
@@ -77,18 +77,18 @@ sub invalid_entry_order_inner_page_test
$node->safe_psql(
'postgres', qq(
DROP TABLE IF EXISTS $relname;
- CREATE TABLE $relname (a text[]);
- CREATE INDEX $indexname ON $relname USING gin (a);
+ CREATE TABLE $relname (a text[]);
+ CREATE INDEX $indexname ON $relname USING gin (a);
INSERT INTO $relname (a) VALUES (('{' || 'pppppppppp' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'qqqqqqqqqq' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'rrrrrrrrrr' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'ssssssssss' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'tttttttttt' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'uuuuuuuuuu' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'vvvvvvvvvv' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'wwwwwwwwww' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'qqqqqqqqqq' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'rrrrrrrrrr' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'ssssssssss' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'tttttttttt' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'uuuuuuuuuu' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'vvvvvvvvvv' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'wwwwwwwwww' || random_string(1870) ||'}')::text[]);
SELECT gin_clean_pending_list('$indexname');
- ));
+ ));
my $relpath = relation_filepath($indexname);
$node->stop;
@@ -118,11 +118,11 @@ sub invalid_entry_columns_order_test
$node->safe_psql(
'postgres', qq(
DROP TABLE IF EXISTS $relname;
- CREATE TABLE $relname (a text[],b text[]);
- CREATE INDEX $indexname ON $relname USING gin (a,b);
+ CREATE TABLE $relname (a text[],b text[]);
+ CREATE INDEX $indexname ON $relname USING gin (a,b);
INSERT INTO $relname (a,b) VALUES ('{aaa}','{bbb}');
SELECT gin_clean_pending_list('$indexname');
- ));
+ ));
my $relpath = relation_filepath($indexname);
$node->stop;
@@ -169,15 +169,15 @@ sub inconsistent_with_parent_key__parent_key_corrupted_test
$node->safe_psql(
'postgres', qq(
DROP TABLE IF EXISTS $relname;
- CREATE TABLE $relname (a text[]);
- CREATE INDEX $indexname ON $relname USING gin (a);
+ CREATE TABLE $relname (a text[]);
+ CREATE INDEX $indexname ON $relname USING gin (a);
INSERT INTO $relname (a) VALUES (('{' || 'llllllllll' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'mmmmmmmmmm' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'nnnnnnnnnn' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'xxxxxxxxxx' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'yyyyyyyyyy' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'mmmmmmmmmm' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'nnnnnnnnnn' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'xxxxxxxxxx' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'yyyyyyyyyy' || random_string(1870) ||'}')::text[]);
SELECT gin_clean_pending_list('$indexname');
- ));
+ ));
my $relpath = relation_filepath($indexname);
$node->stop;
@@ -207,13 +207,13 @@ sub inconsistent_with_parent_key__child_key_corrupted_test
$node->safe_psql(
'postgres', qq(
DROP TABLE IF EXISTS $relname;
- CREATE TABLE $relname (a text[]);
- CREATE INDEX $indexname ON $relname USING gin (a);
+ CREATE TABLE $relname (a text[]);
+ CREATE INDEX $indexname ON $relname USING gin (a);
INSERT INTO $relname (a) VALUES (('{' || 'llllllllll' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'mmmmmmmmmm' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'nnnnnnnnnn' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'xxxxxxxxxx' || random_string(1870) ||'}')::text[]);
- INSERT INTO $relname (a) VALUES (('{' || 'yyyyyyyyyy' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'mmmmmmmmmm' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'nnnnnnnnnn' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'xxxxxxxxxx' || random_string(1870) ||'}')::text[]);
+ INSERT INTO $relname (a) VALUES (('{' || 'yyyyyyyyyy' || random_string(1870) ||'}')::text[]);
SELECT gin_clean_pending_list('$indexname');
));
my $relpath = relation_filepath($indexname);
@@ -248,7 +248,7 @@ sub inconsistent_with_parent_key__parent_key_corrupted_posting_tree_test
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;
@@ -287,7 +287,8 @@ sub relation_filepath
return "$pgdata/$rel";
}
-sub string_replace_block {
+sub string_replace_block
+{
my ($filename, $find, $replace, $blksize, $blkno) = @_;
my $fh;
@@ -310,4 +311,4 @@ sub string_replace_block {
return;
}
-done_testing();
\ No newline at end of file
+done_testing();
--
2.49.0
From e7d488efd2b93ad56573fa597f2134df6e48b0ea Mon Sep 17 00:00:00 2001
From: Arseniy Mukhin <arseniy.mukhin....@gmail.com>
Date: Thu, 29 May 2025 14:33:37 +0300
Subject: [PATCH v4 3/3] undo
---
contrib/amcheck/verify_gin.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/contrib/amcheck/verify_gin.c b/contrib/amcheck/verify_gin.c
index 427cf1669a6..8f6a5410cb7 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 (i == maxoff && ItemPointerIsValid(&stack->parentkey) &&
+ if (stack->parentblk != InvalidBlockNumber && i == maxoff &&
ItemPointerCompare(&stack->parentkey, &posting_item->key) < 0)
ereport(ERROR,
(errcode(ERRCODE_INDEX_CORRUPTED),
@@ -359,10 +359,14 @@ gin_check_posting_tree_parent_keys_consistency(Relation rel, BlockNumber posting
ptr->depth = stack->depth + 1;
/*
- * The rightmost parent key is always invalid item pointer.
- * Its value is 'Infinity' and not explicitly stored.
+ * Set rightmost parent key to invalid item pointer. Its value
+ * is 'Infinity' and not explicitly stored.
*/
- ptr->parentkey = posting_item->key;
+ if (rightlink == InvalidBlockNumber)
+ ItemPointerSetInvalid(&ptr->parentkey);
+ else
+ ptr->parentkey = posting_item->key;
+
ptr->parentblk = stack->blkno;
ptr->blkno = BlockIdGetBlockNumber(&posting_item->child_blkno);
ptr->next = stack->next;
--
2.49.0
