| From: | Nikolay Samokhvalov <nik(at)postgres(dot)ai> |
|---|---|
| To: | Roman Khapov <rkhapov(at)yandex-team(dot)ru> |
| Cc: | Japin Li <japinli(at)hotmail(dot)com>, Kirill Reshke <reshkekirill(at)gmail(dot)com>, Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Miłosz Bieniek <bieniek(dot)milosz(at)proton(dot)me>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: amcheck: support for GiST |
| Date: | 2026-01-31 23:32:06 |
| Message-ID: | CAM527d-siJnFzSQWr5xf0UYB8MfRCNCNoyj7qyD0-Ja-rp_=yA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sat, Jan 31, 2026 at 2:17 PM Roman Khapov <rkhapov(at)yandex-team(dot)ru> wrote:
> >
> > Yeah, verify_common.c does require the header, but what I meant was that
> > verify_nbtree.c no longer needs it.
>
> Oh, thanks, now I see..
>
> Updated the patches.
>
I additionally tested the latest version (v2026-01-12-v2) -- and all looked
good to me.
testing performed:
1. Applied cleanly; built with --enable-cassert, make -C contrib/amcheck
check — all tests pass
2. Tested for 9 core GiST opclasses, on larger indexes (10M records), with
both heapallindexed=false and heapallindexed=true
(10M records, old macbook m1):
Opclass heapallindexed=false heapallindexed=true
-------------- -------------------- -------------------
point_ops 3.9s 5.9s
box_ops 6.3s 9.3s
circle_ops 5.5s 10.6s
poly_ops 5.8s 13.7s
inet_ops 0.6s 5.6s
range_ops 3.2s 6.9s
multirange_ops 2.4s 10.8s
tsvector_ops 3.8s 13.7s
tsquery_ops 0.3s 4.9s
3. Similarly, just to double-check, for extensions that use GiST:
- contrib modules tested: btree_gist, cube, hstore, ltree, pg_trgm, seg
(tested a single opclass for each; so 8 out of 30+ opclasses)
- skipped: intarray because it was annoyingly slow to build (looks like
O(n²)?)
4. Verified corruption detection works. Reproduction steps:
Setup (with data checksums disabled)
create table t as select point(random()*1000, random()*1000) c from
generate_series(1,50000);
create index t_idx on t using gist(c);
select pg_relation_filepath('t_idx'); -- e.g., base/5/16398
Stop server, corrupt index, start:
pg_ctl stop -D $PGDATA
python3 -c "
f = open('$PGDATA/base/5/16398', 'r+b')
f.seek(8192 + 24) # page 1, line pointer area
f.write(b'\xDE\xAD\xBE\xEF' * 8)
f.close()
"
pg_ctl start -D $PGDATA
And then we see an error as expected:
select gist_index_check('t_idx', false);
-- ERROR: invalid line pointer storage in index "t_idx"
-- DETAIL: Index tid=(1,20) lp_off=0, lp_len=0 lp_flags=0.
*****
Side note (general amcheck observation, not specific to this patch): the
existing amcheck TAP tests for heapam include corruption injection tests
(see 001_verify_heapam.pl). Similar tests for index checkers would be
valuable for amcheck -- but I think it's a broader effort beyond this
particular patch's scope.
The patch looks solid.
Nik
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Maciek Sakrejda | 2026-01-31 22:55:52 | Re: V18 change on EXPLAIN ANALYZE |