Re: Amcheck verification of GiST and GIN

2024-08-05 Thread Tomas Vondra
Hi, I've spent a bit more time looking at the GiST part as part of my "parallel GiST build" patch nearby, and I think there's some sort of memory leak. Consider this: create table t (a text); insert into t select md5(i::text) from generate_series(1,2500) s(i); create index on t u

Re: Amcheck verification of GiST and GIN

2024-07-14 Thread Andrey M. Borodin
Hi Tomas! Thank you so much for your interest in the patchset. > On 10 Jul 2024, at 19:01, Tomas Vondra wrote: > > I realized amcheck GIN/GiST support would be useful for testing my > patches adding parallel builds for these index types, so I decided to > take a look at this and do an initial r

Re: Amcheck verification of GiST and GIN

2024-07-12 Thread Tomas Vondra
OK, one more issue report. I originally thought it's a bug in my patch adding parallel builds for GIN indexes, but it turns out it happens even with serial builds on master ... If I build any GIN index, and then do gin_index_parent_check() on it, I get this error: create index jsonb_hash on messa

Re: Amcheck verification of GiST and GIN

2024-07-12 Thread Tomas Vondra
OK, one mere comment - it seems the 0001 patch has incorrect indentation in bt_index_check_callback, triggering this: -- verify_nbtree.c: In function ‘bt_index_check_callback’: verify_nbtree.c:331:25: warning: this ‘if’ clause doe

Re: Amcheck verification of GiST and GIN

2024-07-12 Thread Tomas Vondra
On 7/10/24 18:01, Tomas Vondra wrote: > ... > > That's all for now. I'll add this to the stress-testing tests of my > index build patches, and if that triggers more issues I'll report those. > As mentioned a couple days ago, I started using this patch to validate the patches adding parallel build

Re: Amcheck verification of GiST and GIN

2024-03-10 Thread Andrey M. Borodin
> On 20 Jan 2024, at 07:46, vignesh C wrote: > > I have changed the status of the commitfest entry to "Waiting on > Author" as there was no follow-up on Alexander's queries. Feel free to > address them and change the commitfest entry accordingly. Thanks Vignesh! At the moment it’s obvious th

Re: Amcheck verification of GiST and GIN

2024-01-19 Thread vignesh C
On Mon, 27 Mar 2023 at 03:47, Andrey Borodin wrote: > > On Sun, Mar 19, 2023 at 4:00 PM Andrey Borodin wrote: > > > > Also, there are INCLUDEd attributes. Right now we just put them as-is > > to the bloom filter. Does this constitute a TOAST bug as in B-tree? > > If so, I think we should use a ve

Re: Amcheck verification of GiST and GIN

2023-04-05 Thread Alexander Lakhin
Hi Andrey, 27.03.2023 01:17, Andrey Borodin wrote: I've ported the B-tree TOAST test to GiST, and, as expected, it fails. Finds non-indexed tuple for a fresh valid index. I've tried to use this feature with the latest patch set and discovered that modified pg_amcheck doesn't find any gist inde

Re: Amcheck verification of GiST and GIN

2023-03-26 Thread Peter Geoghegan
On Sun, Mar 19, 2023 at 4:00 PM Andrey Borodin wrote: > After several attempts to corrupt GiST with this 0.01 epsilon > adjustment tolerance I think GiST indexing of points is valid. > Because intersection for search purposes is determined with the same epsilon! > So it's kind of odd > postgre

Re: Amcheck verification of GiST and GIN

2023-03-17 Thread Andrey Borodin
Hi Peter, Thanks for the feedback! I'll work on it during the weekend. On Thu, Mar 16, 2023 at 6:23 PM Peter Geoghegan wrote: > > existence of a "same" routine hints at some confusion about "equality > versus equivalence" issues. Hmm...yes, actually, GiST deals with floats routinely. And there

Re: Amcheck verification of GiST and GIN

2023-03-16 Thread Peter Geoghegan
On Thu, Mar 16, 2023 at 4:48 PM Peter Geoghegan wrote: > Some feedback on the GiST patch: I see that the Bloom filter that's used to implement heapallindexed verification fingerprints index tuples that are formed via calls to gistFormTuple(), without any attempt to normalize-away differences in T

Re: Amcheck verification of GiST and GIN

2023-03-16 Thread Peter Geoghegan
On Sun, Feb 5, 2023 at 4:45 PM Andrey Borodin wrote: > Here's v24 == (v23 + a step for pg_amcheck). There's a lot of > shotgun-style changes, but I hope next index types will be easy to add > now. Some feedback on the GiST patch: * You forgot to initialize GistCheckState.heaptuplespresent to 0.

Re: Amcheck verification of GiST and GIN

2023-02-22 Thread Michael Banck
Hi, On Thu, Feb 02, 2023 at 12:56:47PM -0800, Nikolay Samokhvalov wrote: > On Thu, Feb 2, 2023 at 12:43 PM Peter Geoghegan wrote: > > I think that that problem should be solved at a higher level, in the > > program that runs amcheck. Note that pg_amcheck will already do this > > for B-Tree indexe

Re: Amcheck verification of GiST and GIN

2023-02-04 Thread Andrey Borodin
Thank for working on this, Peter! On Fri, Feb 3, 2023 at 6:50 PM Peter Geoghegan wrote: > > I think that we should focus on getting the GiST patch into shape for > commit first, since that seems easier. > Here's the next version. I've focused on GiST part in this revision. Changes: 1. Refactored

Re: Amcheck verification of GiST and GIN

2023-02-03 Thread Peter Geoghegan
On Thu, Feb 2, 2023 at 12:15 PM Peter Geoghegan wrote: > * Why are there only WARNINGs, never ERRORs here? Attached revision v22 switches all of the WARNINGs over to ERRORs. It has also been re-indented, and now uses a non-generic version of PageGetItemIdCareful() in both verify_gin.c and verify_

Re: Amcheck verification of GiST and GIN

2023-02-02 Thread Peter Geoghegan
On Thu, Feb 2, 2023 at 12:56 PM Nikolay Samokhvalov wrote: > I already realized my mistake – indeed, having multiple errors for 1 index > doesn't seem to be super practically helpful. I wouldn't mind supporting it if the cost wasn't too high. But I believe that it's not a good trade-off. >> I th

Re: Amcheck verification of GiST and GIN

2023-02-02 Thread Nikolay Samokhvalov
On Thu, Feb 2, 2023 at 12:43 PM Peter Geoghegan wrote: > I agree that this matters at the level of whole indexes. > I already realized my mistake – indeed, having multiple errors for 1 index doesn't seem to be super practically helpful. > I think that that problem should be solved at a higher

Re: Amcheck verification of GiST and GIN

2023-02-02 Thread Peter Geoghegan
On Thu, Feb 2, 2023 at 12:31 PM Nikolay Samokhvalov wrote: > I understand your thoughts (I think) and agree with them, but at least one > scenario where I do want to see *all* errors is corruption prevention – > running > amcheck in lower environments, not in production, to predict and prevent >

Re: Amcheck verification of GiST and GIN

2023-02-02 Thread Nikolay Samokhvalov
On Thu, Feb 2, 2023 at 12:15 PM Peter Geoghegan wrote: > On Thu, Feb 2, 2023 at 11:51 AM Peter Geoghegan wrote: > ... > Admittedly there is some value in seeing multiple WARNINGs to true > experts that are performing some kind of forensic analysis, but that > doesn't seem worth it to me -- I'm

Re: Amcheck verification of GiST and GIN

2023-02-02 Thread Peter Geoghegan
On Thu, Feb 2, 2023 at 11:51 AM Peter Geoghegan wrote: > I also have some questions about the verification functionality itself: I forgot to include another big concern here: * Why are there only WARNINGs, never ERRORs here? It's far more likely that you'll run into problems when running amchec

Re: Amcheck verification of GiST and GIN

2023-02-02 Thread Peter Geoghegan
On Fri, Jan 13, 2023 at 8:15 PM Andrey Borodin wrote: > (v21 of patch series) I can see why the refactoring patch is necessary overall, but I have some concerns about the details. More specifically: * PageGetItemIdCareful() doesn't seem like it needs to be moved to amcheck.c and generalized to w

Re: Amcheck verification of GiST and GIN

2023-01-30 Thread Aleksander Alekseev
Hi Andrey, > Thanks! I also found out that there was a CI complaint about amcheck.h > not including some necessary stuff. Here's a version with a fix for > that. Thanks for the updated patchset. One little nitpick I have is that the tests cover only cases when all the checks pass successfully. T

Re: Amcheck verification of GiST and GIN

2023-01-13 Thread Andrey Borodin
On Fri, Jan 13, 2023 at 7:35 PM Jose Arthur Benetasso Villanova wrote: > > Hello again. I see the change. Thanks > Thanks! I also found out that there was a CI complaint about amcheck.h not including some necessary stuff. Here's a version with a fix for that. Best regards, Andrey Borodin. v21-

Re: Amcheck verification of GiST and GIN

2023-01-13 Thread Jose Arthur Benetasso Villanova
On Fri, 13 Jan 2023, Andrey Borodin wrote: On Fri, Jan 13, 2023 at 3:46 AM Jose Arthur Benetasso Villanova wrote: The only thing that I found is the gin_index_parent_check function in docs still references the "gin_index_parent_check(index regclass, heapallindexed boolean) returns void"

Re: Amcheck verification of GiST and GIN

2023-01-13 Thread Andrey Borodin
On Fri, Jan 13, 2023 at 3:46 AM Jose Arthur Benetasso Villanova wrote: > > The only thing that I found is the gin_index_parent_check function in docs > still references the "gin_index_parent_check(index regclass, > heapallindexed boolean) returns void" > Correct! Please find the attached fixed ve

Re: Amcheck verification of GiST and GIN

2023-01-13 Thread Jose Arthur Benetasso Villanova
On Sun, 8 Jan 2023, Andrey Borodin wrote: On Sun, Jan 8, 2023 at 8:05 PM Andrey Borodin wrote: Please find the attached new version. In this patchset heapallindexed flag is removed from GIN checks. Uh... sorry, git-formatted wrong branch. Here's the correct version. Double checked. Hel

Re: Amcheck verification of GiST and GIN

2023-01-08 Thread Andrey Borodin
On Sun, Jan 8, 2023 at 8:05 PM Andrey Borodin wrote: > > Please find the attached new version. In this patchset heapallindexed > flag is removed from GIN checks. > Uh... sorry, git-formatted wrong branch. Here's the correct version. Double checked. Best regards, Andrey Borodin. v19-0003-Add-gin

Re: Amcheck verification of GiST and GIN

2023-01-08 Thread Andrey Borodin
Hi Jose, thank you for review and sorry for so long delay to answer. On Wed, Dec 14, 2022 at 4:19 AM Jose Arthur Benetasso Villanova wrote: > > > On Sun, 27 Nov 2022, Andrey Borodin wrote: > > > On Sun, Nov 27, 2022 at 1:29 PM Andrey Borodin > > wrote: > >> > > I was wrong. GIN check does simil

Re: Amcheck verification of GiST and GIN

2022-12-14 Thread Robert Haas
On Wed, Dec 14, 2022 at 7:19 AM Jose Arthur Benetasso Villanova wrote: > I'm a bit lost here. I tried your patch again and indeed the > heapallindexed inside gin_check_parent_keys_consistency has a TODO > comment, but it's unclear to me if you are going to implement it or if the > patch "needs rev

Re: Amcheck verification of GiST and GIN

2022-12-14 Thread Jose Arthur Benetasso Villanova
On Sun, 27 Nov 2022, Andrey Borodin wrote: On Sun, Nov 27, 2022 at 1:29 PM Andrey Borodin wrote: I was wrong. GIN check does similar gin_refind_parent() to lock pages in bottom-up manner and truly verify downlink-child_page invariant. Does this mean that we need the adjustment in docs?

Re: Amcheck verification of GiST and GIN

2022-11-27 Thread Andrey Borodin
On Sun, Nov 27, 2022 at 1:29 PM Andrey Borodin wrote: > > GiST verification checks only one invariant that can be verified if > page locks acquired the same way as page split does. > GIN does not require ShareLock because it does not check cross-level > invariants. > I was wrong. GIN check does

Re: Amcheck verification of GiST and GIN

2022-11-27 Thread Andrey Borodin
Hello! Thank you for the review! On Thu, Nov 24, 2022 at 6:04 PM Jose Arthur Benetasso Villanova wrote: > > It compiled with those 2 warnings: > > verify_gin.c: In function 'gin_check_parent_keys_consistency': > verify_gin.c:481:38: warning: declaration of 'maxoff' shadows a previous > local [-W

Re: Amcheck verification of GiST and GIN

2022-11-24 Thread Jose Arthur Benetasso Villanova
Hello. I reviewed this patch and I would like to share some comments. It compiled with those 2 warnings: verify_gin.c: In function 'gin_check_parent_keys_consistency': verify_gin.c:481:38: warning: declaration of 'maxoff' shadows a previous local [-Wshadow=compatible-local] 481 |

Re: Amcheck verification of GiST and GIN

2022-10-08 Thread Andrew Borodin
On Sun, Oct 2, 2022 at 12:12 AM Andres Freund wrote: > > Here's an updated patch adding meson compat. Thank you, Andres! Here's one more rebase (something was adjusted in amcheck build). Also I've fixed new warnings except warning about absent heapallindexed for GIN. It's a TODO. Thanks! Best r

Re: Amcheck verification of GiST and GIN

2022-10-02 Thread Andres Freund
Hi, On 2022-09-22 08:19:09 -0700, Andres Freund wrote: > Hi, > > On 2022-08-17 17:28:02 +0500, Andrey Borodin wrote: > > Here's v13. Changes: > > 1. Fixed passing through downlink in GIN index > > 2. Fixed GIN tests (one test case was not working) > > > > Thanks to Vitaliy Kukharik for trying th

Re: Amcheck verification of GiST and GIN

2022-09-22 Thread Andres Freund
Hi, On 2022-08-17 17:28:02 +0500, Andrey Borodin wrote: > Here's v13. Changes: > 1. Fixed passing through downlink in GIN index > 2. Fixed GIN tests (one test case was not working) > > Thanks to Vitaliy Kukharik for trying this patches. Due to the merge of the meson based build, this patch needs

Re: Amcheck verification of GiST and GIN

2022-08-17 Thread Andrey Borodin
> On 23 Jul 2022, at 14:40, Andrey Borodin wrote: > > Done. PFA attached patchset. > > Best regards, Andrey Borodin. > Here's v13. Changes: 1. Fixed passing through downlink in GIN index 2. Fixed GIN tests (one test case was not working) Thanks to Vitaliy Kukharik for trying this patches.

Re: Amcheck verification of GiST and GIN

2022-07-23 Thread Andrey Borodin
> On 26 Jun 2022, at 00:10, Andrey Borodin wrote: > > I will split the patch in 3 steps: > 1. extract generic functions to amcheck.c > 2. add gist functions > 3. add gin functions > > I'll fix other notes too in the next version. Done. PFA attached patchset. Thanks! Best regards, Andrey Bo

Re: Amcheck verification of GiST and GIN

2022-06-25 Thread Andrey Borodin
> On 23 Jun 2022, at 00:27, Nikolay Samokhvalov wrote: > > Since you're talking to yourself, just wanted to support you – this is an > important thing, definitely should be very useful for many projects; I hope > to find time to test it in the next few days. Thanks Nikolay! > On 23 Jun 2

Re: Amcheck verification of GiST and GIN

2022-06-22 Thread Andres Freund
Hi, I think having amcheck for more indexes is great. On 2022-06-22 20:40:56 +0300, Andrey Borodin wrote: >> diff --git a/contrib/amcheck/amcheck.c b/contrib/amcheck/amcheck.c > new file mode 100644 > index 00..7a222719dd > --- /dev/null > +++ b/contrib/amcheck/amcheck.c > @@ -0,0 +1,187

Re: Amcheck verification of GiST and GIN

2022-06-22 Thread Nikolay Samokhvalov
On Wed, Jun 22, 2022 at 11:35 AM Andrey Borodin wrote: > > On 30 May 2022, at 12:40, Andrey Borodin wrote: > > > > What do you think? > > Hi Andrey! > Hi Andrey! Since you're talking to yourself, just wanted to support you – this is an important thing, definitely should be very useful for many

Re: Amcheck verification of GiST and GIN

2022-06-22 Thread Andrey Borodin
> On 30 May 2022, at 12:40, Andrey Borodin wrote: > > What do you think? Hi Andrey! Here's a version with better tests. I've made sure that GiST tests actually trigger page reuse after deletion. And enhanced comments in both GiST and GIN test scripts. I hope you'll like it. GIN heapallinde

Amcheck verification of GiST and GIN

2022-05-30 Thread Andrey Borodin
Hello world! Few years ago we had a thread with $subj [0]. A year ago Heikki put a lot of effort in improving GIN checks [1] while hunting a GIN bug. And in view of some releases with a recommendation to reindex anything that fails or lacks amcheck verification, I decided that I want to review t