Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index.

Hi, hackers!

On Wed, 24 Apr 2024 at 13:58, Alexander Korotkov <aekorotkov(at)gmail(dot)com>
wrote:

> On Wed, Apr 17, 2024 at 9:38 AM Peter Eisentraut <peter(at)eisentraut(dot)org>
> wrote:
> > On 24.10.23 22:13, Alexander Korotkov wrote:
> > > On Wed, Sep 28, 2022 at 11:44 AM Aleksander Alekseev
> > > <aleksander(at)timescale(dot)com> wrote:
> > >>> I think, this patch was marked as "Waiting on Author", probably, by
> mistake. Since recent changes were done without any significant code
> changes and CF bot how happy again.
> > >>>
> > >>> I'm going to move it to RfC, could I? If not, please tell why.
> > >>
> > >> I restored the "Ready for Committer" state. I don't think it's a good
> > >> practice to change the state every time the patch has a slight
> > >> conflict or something. This is not helpful at all. Such things happen
> > >> quite regularly and typically are fixed in a couple of days.
> > >
> > > This patch seems useful to me. I went through the thread, it seems
> > > that all the critics are addressed.
> > >
> > > I've rebased this patch. Also, I've run perltidy for tests, split
> > > long errmsg() into errmsg(), errdetail() and errhint(), and do other
> > > minor enchantments.
> > >
> > > I think this patch is ready to go. I'm going to push it if there are
> > > no objections.
> >
> > I just found the new pg_amcheck option --checkunique in PG17-to-be.
> > Could we rename this to --check-unique? Seems friendlier. Maybe also
> > rename the bt_index_check function argument to check_unique.
>
> +1 from me
> Let's do so if nobody objects.
>

Thank you very much for your input in this thread!

See the patches based on the proposals in the attachment:

0001: Optimize speed by avoiding heap visibility checking for different
non-deduplicated index tuples as proposed by Noah Misch

Speed measurements on my laptop using the exact method recommended by Noah
upthread:
Current master branch: checkunique off: 144s, checkunique on: 419s
With patch 0001: checkunique off: 141s, checkunique on: 171s

0002: Use structure to store and transfer info about last visible heap
entry (code refactoring) as proposed by Alexander Korotkov

0003: Don't load rightpage into BtreeCheckState (code refactoring) as
proposed by Peter Geoghegan

Loading of right page for cross-page unique constraint check in the same
way as in bt_right_page_check_scankey()

0004: Report error when next page to a leaf is not a leaf as proposed by
Peter Geoghegan

I think it's a very improbable condition and this check might be not
necessary, but it's right and safe to break check and report error.

0005: Rename checkunique parameter to more user friendly as proposed by
Peter Eisentraut and Alexander Korotkov

Again many thanks for the useful proposals!

Regards,
Pavel Borisov,
Supabase