MIME-Version: 1.0
References: 
 <CALT9ZEHRn5xAM5boga0qnrCmPV52bScEK2QnQ1HmUZDD301JEg@mail.gmail.com>
 <E91B88AB-E3A1-42A7-9BE2-40805AE5F35C@enterprisedb.com>
 <CALT9ZEFYsYPms0SPp4ZxpFCOBQbgE+iopoVKzU8Pni8Y6+RKtQ@mail.gmail.com>
 <6A958AE0-CFB9-4B1F-8EF8-BF6E729AFB59@enterprisedb.com>
In-Reply-To: <6A958AE0-CFB9-4B1F-8EF8-BF6E729AFB59@enterprisedb.com>
From: Pavel Borisov <pashkin.elfe@gmail.com>
Date: Tue, 2 Mar 2021 00:23:23 +0400
Message-ID: 
 <CALT9ZEGgKAXDfVaAjBjBf4e6-L_4NVQsJ+7gzHd0JLwNxjXTFA@mail.gmail.com>
Subject: Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree
 index.
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>,
	Postgres hackers <pgsql-hackers@lists.postgresql.org>,
 Peter Geoghegan <pg@bowt.ie>
Content-Type: multipart/alternative; boundary="0000000000006e08f805bc7f6487"
Precedence: bulk

--0000000000006e08f805bc7f6487
Content-Type: text/plain; charset="UTF-8"

>
> If bt_index_check() and bt_index_parent_check() are to have this
> functionality, shouldn't there be an option controlling it much as the
> option (heapallindexed boolean) controls checking whether all entries in
> the heap are indexed in the btree?  It seems inconsistent to have an option
> to avoid checking the heap for that, but not for this.  Alternately, this
> might even be better coded as its own function, named something like
> bt_unique_index_check() perhaps.  I hope Peter might advise?
>

As for heap checking, my reasoning was that we can not check whether a
unique constraint violated by the index, without checking heap tuple
visibility. I.e. we can have many identical index entries without
uniqueness violated if only one of them corresponds to a visible heap
tuple. So heap checking included in my patch is _necessary_ for unique
constraint checking, it should not have an option to be disabled,
otherwise, the only answer we can get is that unique constraint MAY be
violated and may not be, which is quite useless. If you meant something
different, please elaborate.

I can try to rewrite unique constraint checking to be done after all others
check but I suppose it's the performance considerations are that made
previous amcheck routines to do many checks simultaneously. I tried to
stick to this practice. It's also not so elegant to duplicate much code to
make uniqueness checks independently and the resulting patch will be much
bigger and harder to review.

Anyway, your and Peter's further considerations are always welcome.

-- 
Best regards,
Pavel Borisov

Postgres Professional: http://postgrespro.com <http://www.postgrespro.com>

--0000000000006e08f805bc7f6487
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div class=3D"gmail_quote"><blockquote class=3D"gmail_quot=
e" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204)=
;padding-left:1ex">If bt_index_check() and bt_index_parent_check() are to h=
ave this functionality, shouldn&#39;t there be an option controlling it muc=
h as the option (heapallindexed boolean) controls checking whether all entr=
ies in the heap are indexed in the btree?=C2=A0 It seems inconsistent to ha=
ve an option to avoid checking the heap for that, but not for this.=C2=A0 A=
lternately, this might even be better coded as its own function, named some=
thing like bt_unique_index_check() perhaps.=C2=A0 I hope Peter might advise=
?<br></blockquote><div>=C2=A0</div><div>As for heap checking, my reasoning =
was that we can not check whether a unique constraint violated by the index=
, without checking heap tuple visibility. I.e. we can have many identical i=
ndex entries without uniqueness violated if only one of them corresponds to=
 a visible heap tuple. So heap checking included in my patch is _necessary_=
 for unique constraint checking, it should not have an option to be disable=
d, otherwise, the only answer we can get is that unique=C2=A0constraint MAY=
 be violated and may not be, which is quite useless. If you meant something=
 different, please elaborate.=C2=A0</div><div><br></div><div>I can try to r=
ewrite unique constraint checking to be done after all others check but I s=
uppose it&#39;s the performance considerations are that made previous amche=
ck routines to do many checks simultaneously. I tried to stick to this prac=
tice. It&#39;s also not so elegant to duplicate much code to make uniquenes=
s checks independently and the resulting patch will be much bigger and hard=
er to review.</div><div><br></div><div>Anyway, your and Peter&#39;s further=
 considerations are always welcome.</div><div><br></div></div>-- <br><div d=
ir=3D"ltr" class=3D"gmail_signature" data-smartmail=3D"gmail_signature"><di=
v dir=3D"ltr"><div>Best regards,<br>Pavel Borisov<br><br>Postgres Professio=
nal: <a href=3D"http://www.postgrespro.com" rel=3D"noopener noreferrer" tar=
get=3D"_blank">http://postgrespro.com</a><br></div></div></div></div>

--0000000000006e08f805bc7f6487--