Hi all.
I was reviewing nearby thread about parallel index creation for GIN,
and spotted this test [0] :
create table gin_t (a int[]);
insert into gin_t select * from rand_array(3000, 0, 100, 0, 50);
create index on gin_t using gin(a);
v34 fails on this. The reason is we should never check t
> On 26 Nov 2024, at 11:50, Kirill Reshke wrote:
>
> I did mechanical patch rebase & beautification.
Many thanks! Addressing Tomas' feedback was still one of top items on my todo
list. And I'm more than happy that someone advance this patchset.
> Notice my first patch, i did small refactori
Hi!
On Mon, 5 Aug 2024 at 20:05, Tomas Vondra wrote:
>
> 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
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
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
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
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
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
> 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
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
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
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
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
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
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.
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
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
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_
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
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
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
>
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
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
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
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
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-
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"
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
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
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
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
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
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?
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
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
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 |
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
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
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
> 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.
> 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
> 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
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
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
> 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
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
46 matches
Mail list logo
