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

2024-05-17 Thread Pavel Borisov
Hi, Mark! > > At the first glance it's not clear to me: > > - why your test creates cross-page unique constraint violations? > > To see if they are detected. > > > - how do you know they are not detected? > > It appears that they are detected. At least, rerunning the test after > adjusting the

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

2024-05-17 Thread Peter Geoghegan
On Fri, May 17, 2024 at 4:10 PM Mark Dilger wrote: > The quick-and-dirty TAP test I wrote this morning is intended to introduce > duplicates across page boundaries, not to test for ones that got there by > normal database activity. In other words, the TAP test forcibly corrupts the > index by

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

2024-05-17 Thread Mark Dilger
> On May 17, 2024, at 1:00 PM, Peter Geoghegan wrote: > > Many different parts of the B-Tree code will fight against allowing > duplicates of the same value to span multiple leaf pages -- this is > especially true for unique indexes. The quick-and-dirty TAP test I wrote this morning is

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

2024-05-17 Thread Mark Dilger
> On May 17, 2024, at 12:42 PM, Pavel Borisov wrote: > > At the first glance it's not clear to me: > - why your test creates cross-page unique constraint violations? To see if they are detected. > - how do you know they are not detected? It appears that they are detected. At least,

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

2024-05-17 Thread Peter Geoghegan
On Fri, May 17, 2024 at 3:42 PM Mark Dilger wrote: > On further review, the test was not anticipating the error message "high key > invariant violated for index". That wasn't seen in calls to > bt_index_parent_check(), but appears as one of the errors from > bt_index_check(). I am rerunning

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

2024-05-17 Thread Pavel Borisov
Hi, Mark! On Fri, 17 May 2024 at 23:10, Mark Dilger wrote: > > > > On May 17, 2024, at 11:51 AM, Pavel Borisov > wrote: > > > > Amcheck with checkunique option does check uniqueness violation between > pages. But it doesn't warranty detection of cross page uniqueness > violations in extremely

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

2024-05-17 Thread Mark Dilger
> On May 17, 2024, at 12:10 PM, Mark Dilger > wrote: > >> Amcheck with checkunique option does check uniqueness violation between >> pages. But it doesn't warranty detection of cross page uniqueness violations >> in extremely rare cases when the first equal index entry on the next page >>

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

2024-05-17 Thread Mark Dilger
> On May 17, 2024, at 11:51 AM, Pavel Borisov wrote: > > Amcheck with checkunique option does check uniqueness violation between > pages. But it doesn't warranty detection of cross page uniqueness violations > in extremely rare cases when the first equal index entry on the next page >

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

2024-05-17 Thread Pavel Borisov
Hi, Mark! > The documentation in > https://www.postgresql.org/docs/devel/amcheck.html#AMCHECK-FUNCTIONS is > ambiguous: > > "bt_index_check does not verify invariants that span child/parent > relationships, but will verify the presence of all heap tuples as index > tuples within the index when

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

2024-05-17 Thread Mark Dilger
> On May 17, 2024, at 3:11 AM, Alexander Korotkov wrote: > > On Mon, May 13, 2024 at 4:42 AM Alexander Korotkov > wrote: >> On Mon, May 13, 2024 at 12:23 AM Alexander Korotkov >> wrote: >>> On Sat, May 11, 2024 at 4:13 AM Mark Dilger >>> wrote: > On May 10, 2024, at 12:05 PM, Alexander

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

2024-05-17 Thread Pavel Borisov
Hi, Alexander! On Fri, 17 May 2024 at 14:11, Alexander Korotkov wrote: > On Mon, May 13, 2024 at 4:42 AM Alexander Korotkov > wrote: > > On Mon, May 13, 2024 at 12:23 AM Alexander Korotkov > > wrote: > > > On Sat, May 11, 2024 at 4:13 AM Mark Dilger > > > wrote: > > > > > On May 10, 2024, at

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

2024-05-17 Thread Alexander Korotkov
On Mon, May 13, 2024 at 4:42 AM Alexander Korotkov wrote: > On Mon, May 13, 2024 at 12:23 AM Alexander Korotkov > wrote: > > On Sat, May 11, 2024 at 4:13 AM Mark Dilger > > wrote: > > > > On May 10, 2024, at 12:05 PM, Alexander Korotkov > > > > wrote: > > > > The only bt_target_page_check()

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

2024-05-13 Thread Pavel Borisov
A correction of a typo in previous message: non-leaf pages iteration cycles (under P_ISLEAF(topaque)) -> non-leaf pages iteration cycles (under !P_ISLEAF(topaque)) On Mon, 13 May 2024 at 16:19, Pavel Borisov wrote: > > > On Mon, 13 May 2024 at 15:55, Pavel Borisov > wrote: > >> Hi, Alexander!

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

2024-05-13 Thread Pavel Borisov
On Mon, 13 May 2024 at 15:55, Pavel Borisov wrote: > Hi, Alexander! > > On Mon, 13 May 2024 at 05:42, Alexander Korotkov > wrote: > >> On Mon, May 13, 2024 at 12:23 AM Alexander Korotkov >> wrote: >> > On Sat, May 11, 2024 at 4:13 AM Mark Dilger >> > wrote: >> > > > On May 10, 2024, at 12:05

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

2024-05-13 Thread Pavel Borisov
Hi, Alexander! On Mon, 13 May 2024 at 05:42, Alexander Korotkov wrote: > On Mon, May 13, 2024 at 12:23 AM Alexander Korotkov > wrote: > > On Sat, May 11, 2024 at 4:13 AM Mark Dilger > > wrote: > > > > On May 10, 2024, at 12:05 PM, Alexander Korotkov < > aekorot...@gmail.com> wrote: > > > >

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

2024-05-12 Thread Alexander Korotkov
On Mon, May 13, 2024 at 12:23 AM Alexander Korotkov wrote: > On Sat, May 11, 2024 at 4:13 AM Mark Dilger > wrote: > > > On May 10, 2024, at 12:05 PM, Alexander Korotkov > > > wrote: > > > The only bt_target_page_check() caller is > > > bt_check_level_from_leftmost(), which overrides

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

2024-05-12 Thread Alexander Korotkov
On Sat, May 11, 2024 at 4:13 AM Mark Dilger wrote: > > On May 10, 2024, at 12:05 PM, Alexander Korotkov > > wrote: > > The only bt_target_page_check() caller is > > bt_check_level_from_leftmost(), which overrides state->target in the > > next iteration anyway. I think the patch is just

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

2024-05-10 Thread Tom Lane
Mark Dilger writes: > Regardless of which of 1..4 you pick, I think it could all do with more > regression test coverage. Indeed. If we have no regression tests that reach this code, it's folly to touch it at all, but most especially so post-feature-freeze. I think the *first* order of

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

2024-05-10 Thread Mark Dilger
> On May 10, 2024, at 12:05 PM, Alexander Korotkov wrote: > > The only bt_target_page_check() caller is > bt_check_level_from_leftmost(), which overrides state->target in the > next iteration anyway. I think the patch is just refactoring to > eliminate the confusion pointer by Peter

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

2024-05-10 Thread Mark Dilger
> On May 10, 2024, at 11:42 AM, Pavel Borisov wrote: > > IMO 0003 doesn't introduce nor fixes a bug. It loads rightpage into a local > variable, rather that to a BtreeCheckState that can have another users of > state->target afterb uniqueness check in the future, but don't have now. So >

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

2024-05-10 Thread Alexander Korotkov
On Fri, May 10, 2024 at 8:35 PM Mark Dilger wrote: > > On May 10, 2024, at 5:10 AM, Pavel Borisov wrote: > > On Fri, 10 May 2024 at 12:39, Alexander Korotkov > > wrote: > > On Fri, May 10, 2024 at 3:43 AM Tom Lane wrote: > > > Alexander Korotkov writes: > > > > The revised patchset is

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

2024-05-10 Thread Pavel Borisov
On Fri, 10 May 2024, 22:42 Pavel Borisov, wrote: > Hi, Mark! > > > On Fri, 10 May 2024, 21:35 Mark Dilger, > wrote: > >> >> >> > On May 10, 2024, at 5:10 AM, Pavel Borisov >> wrote: >> > >> > Hi, Alexander! >> > >> > On Fri, 10 May 2024 at 12:39, Alexander Korotkov >> wrote: >> > On Fri, May

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

2024-05-10 Thread Pavel Borisov
Hi, Mark! On Fri, 10 May 2024, 21:35 Mark Dilger, wrote: > > > > On May 10, 2024, at 5:10 AM, Pavel Borisov > wrote: > > > > Hi, Alexander! > > > > On Fri, 10 May 2024 at 12:39, Alexander Korotkov > wrote: > > On Fri, May 10, 2024 at 3:43 AM Tom Lane wrote: > > > Alexander Korotkov writes:

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

2024-05-10 Thread Mark Dilger
> On May 10, 2024, at 5:10 AM, Pavel Borisov wrote: > > Hi, Alexander! > > On Fri, 10 May 2024 at 12:39, Alexander Korotkov wrote: > On Fri, May 10, 2024 at 3:43 AM Tom Lane wrote: > > Alexander Korotkov writes: > > > The revised patchset is attached. I applied cosmetical changes. I'm >

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

2024-05-10 Thread Pavel Borisov
Hi, Alexander! On Fri, 10 May 2024 at 12:39, Alexander Korotkov wrote: > On Fri, May 10, 2024 at 3:43 AM Tom Lane wrote: > > Alexander Korotkov writes: > > > The revised patchset is attached. I applied cosmetical changes. I'm > > > going to push it if no objections. > > > > Is this really

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

2024-05-10 Thread Alexander Korotkov
On Fri, May 10, 2024 at 3:43 AM Tom Lane wrote: > Alexander Korotkov writes: > > The revised patchset is attached. I applied cosmetical changes. I'm > > going to push it if no objections. > > Is this really suitable material to be pushing post-feature-freeze? > It doesn't look like it's fixing

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

2024-05-10 Thread Pavel Borisov
Hi, Tom! On Fri, 10 May 2024, 04:43 Tom Lane, wrote: > Alexander Korotkov writes: > > The revised patchset is attached. I applied cosmetical changes. I'm > > going to push it if no objections. > > Is this really suitable material to be pushing post-feature-freeze? > It doesn't look like it's

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

2024-05-09 Thread Tom Lane
Alexander Korotkov writes: > The revised patchset is attached. I applied cosmetical changes. I'm > going to push it if no objections. Is this really suitable material to be pushing post-feature-freeze? It doesn't look like it's fixing any new-in-v17 issues. regards,

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

2024-05-09 Thread Alexander Korotkov
On Wed, May 1, 2024 at 5:26 AM Alexander Korotkov wrote: > On Wed, May 1, 2024 at 5:24 AM Noah Misch wrote: > > On Thu, Apr 25, 2024 at 04:59:54PM +0400, Pavel Borisov wrote: > > > 0001: Optimize speed by avoiding heap visibility checking for different > > > non-deduplicated index tuples as

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

2024-04-30 Thread Alexander Korotkov
Hi Noah, On Wed, May 1, 2024 at 5:24 AM Noah Misch wrote: > On Thu, Apr 25, 2024 at 04:59:54PM +0400, Pavel Borisov wrote: > > 0001: Optimize speed by avoiding heap visibility checking for different > > non-deduplicated index tuples as proposed by Noah Misch > > > > Speed measurements on my

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

2024-04-30 Thread Noah Misch
On Thu, Apr 25, 2024 at 04:59:54PM +0400, Pavel Borisov wrote: > 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

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

2024-04-25 Thread Pavel Borisov
Hi, Karina! On Thu, 25 Apr 2024 at 17:44, Karina Litskevich wrote: > Hi, hackers! > > On Thu, Apr 25, 2024 at 4:00 PM Pavel Borisov > wrote: > >> 0005: Rename checkunique parameter to more user friendly as proposed by >> Peter Eisentraut and Alexander Korotkov >> > > I'm not sure renaming

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

2024-04-25 Thread Karina Litskevich
Hi, hackers! On Thu, Apr 25, 2024 at 4:00 PM Pavel Borisov wrote: > 0005: Rename checkunique parameter to more user friendly as proposed by > Peter Eisentraut and Alexander Korotkov > I'm not sure renaming checkunique is a good idea. Other arguments of bt_index_check and bt_index_parent_check

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

2024-04-25 Thread Pavel Borisov
Hi, hackers! On Wed, 24 Apr 2024 at 13:58, Alexander Korotkov wrote: > On Wed, Apr 17, 2024 at 9:38 AM Peter Eisentraut > wrote: > > On 24.10.23 22:13, Alexander Korotkov wrote: > > > On Wed, Sep 28, 2022 at 11:44 AM Aleksander Alekseev > > > wrote: > > >>> I think, this patch was marked as

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

2024-04-24 Thread Alexander Korotkov
On Wed, Apr 17, 2024 at 9:38 AM Peter Eisentraut wrote: > On 24.10.23 22:13, Alexander Korotkov wrote: > > On Wed, Sep 28, 2022 at 11:44 AM Aleksander Alekseev > > wrote: > >>> I think, this patch was marked as "Waiting on Author", probably, by > >>> mistake. Since recent changes were done

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

2024-04-24 Thread Alexander Korotkov
On Wed, Apr 17, 2024 at 6:41 PM Pavel Borisov wrote: >> I did notice (I meant to point out) that I have concerns about this >> part of the new uniqueness check code: >> " >> if (P_IGNORE(topaque) || !P_ISLEAF(topaque)) >> break; >> " >> >> My concern here is with the !P_ISLEAF(topaque) test

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

2024-04-17 Thread Pavel Borisov
I did notice (I meant to point out) that I have concerns about this > part of the new uniqueness check code: > " > if (P_IGNORE(topaque) || !P_ISLEAF(topaque)) > break; > " My concern here is with the !P_ISLEAF(topaque) test -- it shouldn't be > required I agree. But I didn't see the need

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

2024-04-17 Thread Peter Eisentraut
On 24.10.23 22:13, Alexander Korotkov wrote: On Wed, Sep 28, 2022 at 11:44 AM Aleksander Alekseev 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

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

2024-03-29 Thread Noah Misch
On Fri, Mar 29, 2024 at 02:17:08PM -0400, Peter Geoghegan wrote: > On Mon, Mar 25, 2024 at 2:24 PM Noah Misch wrote: > > I wasn't thinking about changing the pre-v17 bt_right_page_check_scankey() > > code. I got interested in this area when I saw the interaction of the new > > "first key on the

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

2024-03-29 Thread Peter Geoghegan
On Mon, Mar 25, 2024 at 2:24 PM Noah Misch wrote: > I wasn't thinking about changing the pre-v17 bt_right_page_check_scankey() > code. I got interested in this area when I saw the interaction of the new > "first key on the next page" logic with bt_right_page_check_scankey(). The > patch made

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

2024-03-25 Thread Noah Misch
On Mon, Mar 25, 2024 at 12:03:10PM -0400, Peter Geoghegan wrote: > On Sun, Mar 24, 2024 at 10:03 PM Noah Misch wrote: > Separately, I now see that the committed patch just reuses the code > that has long been used to check that things are in the correct order > across page boundaries: this is

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

2024-03-25 Thread Peter Geoghegan
On Sun, Mar 24, 2024 at 10:03 PM Noah Misch wrote: > > You're going to have to "couple" buffer locks in the style of > > _bt_check_unique() (as well as keeping a buffer lock on "the first > > leaf page a duplicate might be on" throughout) if you need the test to > > work reliably. > > The amcheck

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

2024-03-24 Thread Noah Misch
On Mon, Oct 30, 2023 at 11:29:04AM +0400, Pavel Borisov wrote: > On Wed, 25 Oct 2023 at 00:13, Alexander Korotkov wrote: > > I think this patch is ready to go. I'm going to push it if there are > > no objections. > It's very good that this long-standing patch is finally committed. Thanks a >

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

2023-10-30 Thread Pavel Borisov
Hi, Alexander! On Wed, 25 Oct 2023 at 00:13, Alexander Korotkov wrote: > > On Wed, Sep 28, 2022 at 11:44 AM Aleksander Alekseev > wrote: > > > I think, this patch was marked as "Waiting on Author", probably, by > > > mistake. Since recent changes were done without any significant code > > >

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

2023-10-24 Thread Alexander Korotkov
On Wed, Sep 28, 2022 at 11:44 AM Aleksander Alekseev 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

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

2022-09-28 Thread Aleksander Alekseev
Hi hackers, > 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"

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

2022-09-28 Thread Maxim Orlov
Hi! 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. -- Best regards, Maxim Orlov.

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

2022-09-27 Thread Maxim Orlov
On Thu, 22 Sept 2022 at 18:13, Andres Freund wrote: > Due to the merge of the meson based build this patch needs to be > adjusted: https://cirrus-ci.com/build/6350479973154816 > > Looks like you need to add amcheck--1.3--1.4.sql to the list of files to be > installed and

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

2022-09-22 Thread Andres Freund
Hi, On 2022-05-20 17:46:38 +0400, Pavel Borisov wrote: > CFbot says v12 patch does not apply. > Rebased. PFA v13. > Your reviews are very much welcome! Due to the merge of the meson based build this patch needs to be adjusted: https://cirrus-ci.com/build/6350479973154816 Looks like you need to

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

2022-09-08 Thread Karina Litskevich
Hi, I also would like to suggest a cosmetic change. In v15 a new field checkunique is added after heapallindexed and before no_btree_expansion fields in struct definition, but in initialisation it is added after no_btree_expansion: --- a/src/bin/pg_amcheck/pg_amcheck.c +++

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

2022-09-07 Thread Dmitry Koval
Hi! I would make two cosmetic changes. 1. I suggest replace description of function bt_report_duplicate() from ``` /* * Prepare and print an error message for unique constrain violation in * a btree index under WARNING level. Also set a flag to report ERROR * at the end of the check. */ ```

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

2022-07-20 Thread Aleksander Alekseev
Hi Pavel, > Rebased. PFA v13. > Your reviews are very much welcome! I noticed that this patch is in "Needs Review" state and it has been stuck for some time now, so I decided to take a look. ``` +SELECT bt_index_parent_check('bttest_a_idx', true, true, true); +SELECT

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

2022-05-20 Thread Pavel Borisov
CFbot says v12 patch does not apply. Rebased. PFA v13. Your reviews are very much welcome! -- Best regards, Pavel Borisov Postgres Professional: http://postgrespro.com From 21fe45c0ae8479e0733bd8caeb5d2a19d715e0d9 Mon Sep 17 00:00:00 2001 From: Pavel Borisov Date:

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

2022-05-11 Thread Pavel Borisov
v11 patch do not apply due to recent code changes. Rebased. PFA v12. Please feel free to check and discuss it. -- Best regards, Pavel Borisov Postgres Professional: http://postgrespro.com v12-0001-Add-option-for-amcheck-and-pg_amcheck-to-check-u.patch

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

2022-04-04 Thread Pavel Borisov
> > This patch was broken by d16773cdc86210493a2874cb0cf93f3883fcda73 "Add > macros in hash and btree AMs to get the special area of their pages" > > If it's really just a few macros it should be easy enough to merge but > it would be good to do a rebase given the number of other commits > since

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

2022-04-02 Thread Greg Stark
This patch was broken by d16773cdc86210493a2874cb0cf93f3883fcda73 "Add macros in hash and btree AMs to get the special area of their pages" If it's really just a few macros it should be easy enough to merge but it would be good to do a rebase given the number of other commits since February

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

2022-02-21 Thread Maxim Orlov
I've updated the patch due to recent changes by Daniel Gustafsson (549ec201d6132b7). -- Best regards, Maxim Orlov. v10-0001-Add-option-for-amcheck-and-pg_amcheck-to-check-u.patch Description: Binary data

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

2022-01-13 Thread Pavel Borisov
By the way I've forgotten to add one part of my code into the CF patch related to the treatment of NULL values in checking btree unique constraints. PFA v9 of a patch with this minor code and tests additions. -- Best regards, Pavel Borisov Postgres Professional: http://postgrespro.com

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

2022-01-11 Thread Pavel Borisov
> > The cfbot reports that you have mixed declarations and code > (https://cirrus-ci.com/task/6407449413419008): > > [17:21:26.926] pg_amcheck.c: In function ‘main’: > [17:21:26.926] pg_amcheck.c:634:4: error: ISO C90 forbids mixed > declarations and code [-Werror=declaration-after-statement] >

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

2022-01-11 Thread Julien Rouhaud
Hi, On Fri, Dec 24, 2021 at 2:06 AM Максим Орлов wrote: > > Thanks for your review! Fixed all these remaining things from patch v6. > PFA v7 patch. The cfbot reports that you have mixed declarations and code (https://cirrus-ci.com/task/6407449413419008): [17:21:26.926] pg_amcheck.c: In

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

2021-12-23 Thread Максим Орлов
> > The pstrdup is unnecessary but harmless. > > > - use the existing table for uniqueness check in 005_opclass_damage.pl > > It appears you still create a new table, bttest_unique, rather than using > the existing table int4tbl. That's fine. > > > - added tests into 003_check.pl . It is only

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

2021-12-23 Thread Mark Dilger
> On Dec 22, 2021, at 12:01 AM, Pavel Borisov wrote: > > Thank you, Mark! > > In v6 (PFA) I've made the changes on your advice i.e. > > - pg_amcheck with --checkunique option will ignore uniqueness check (with a > warning) if amcheck version in a db is <1.4 and doesn't support the feature.

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

2021-12-22 Thread Pavel Borisov
> > The tests in check_btree.sql no longer create a bttest_unique table, so > the DROP TABLE is surplusage: > > +DROP TABLE bttest_unique; > +ERROR: table "bttest_unique" does not exist > > > The changes in pg_amcheck.c to pass the new checkunique parameter will > likely need to be based on a

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

2021-12-20 Thread Mark Dilger
> On Dec 20, 2021, at 7:37 AM, Pavel Borisov wrote: > > The patch is pgindented and rebased on the current PG master code. Thank you, Pavel. The tests in check_btree.sql no longer create a bttest_unique table, so the DROP TABLE is surplusage: +DROP TABLE bttest_unique; +ERROR: table

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

2021-12-20 Thread Pavel Borisov
> > >> I completely agree that checking uniqueness requires looking at the > heap, but I don't agree that every caller of bt_index_check on an index > wants that particular check to be performed. There are multiple ways in > which an index might be corrupt, and Peter wrote the code to only check

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

2021-04-08 Thread David Steele
On 3/15/21 11:11 AM, Mark Dilger wrote: On Mar 2, 2021, at 6:08 AM, Pavel Borisov wrote: I completely agree that checking uniqueness requires looking at the heap, but I don't agree that every caller of bt_index_check on an index wants that particular check to be performed. There are

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

2021-03-15 Thread Mark Dilger
> On Mar 2, 2021, at 6:08 AM, Pavel Borisov wrote: > > I completely agree that checking uniqueness requires looking at the heap, but > I don't agree that every caller of bt_index_check on an index wants that > particular check to be performed. There are multiple ways in which an index >

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

2021-03-03 Thread Pavel Borisov
> > You're going to have to "couple" buffer locks in the style of > _bt_check_unique() (as well as keeping a buffer lock on "the first > leaf page a duplicate might be on" throughout) if you need the test to > work reliably. But why bother with that? The tool doesn't have to be > 100% perfect at

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

2021-03-02 Thread Peter Geoghegan
On Mon, Feb 8, 2021 at 2:46 AM Pavel Borisov wrote: > Caveat: if the first entry on the next index page has a key equal to the key > on a previous page AND all heap tid's corresponding to this entry are > invisible, currently cross-page check can not detect unique constraint > violation

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

2021-03-02 Thread Peter Geoghegan
On Mon, Mar 1, 2021 at 11:22 AM Mark Dilger wrote: > 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

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

2021-03-02 Thread Pavel Borisov
> > I completely agree that checking uniqueness requires looking at the heap, > but I don't agree that every caller of bt_index_check on an index wants > that particular check to be performed. There are multiple ways in which an > index might be corrupt, and Peter wrote the code to only check

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

2021-03-01 Thread Mark Dilger
> On Mar 1, 2021, at 12:23 PM, Pavel Borisov wrote: > > 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

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

2021-03-01 Thread Pavel Borisov
> > 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 >

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

2021-03-01 Thread Tom Lane
Mark Dilger writes: > Yes, my review was of v2. Updating to v3, I see that the test passes on my > laptop. It still looks brittle to have all the tid values in the test > output, but it does pass. Hm, anyone tried it on 32-bit hardware? regards, tom lane

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

2021-03-01 Thread Mark Dilger
> On Mar 1, 2021, at 12:05 PM, Pavel Borisov wrote: > > The regression test you provided is not portable. I am getting lots of > errors due to differing output of the form "page lsn=0/4DAD7E0". You might > turn this into a TAP test and use a regular expression to check the output. > May I

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

2021-03-01 Thread Pavel Borisov
> > The regression test you provided is not portable. I am getting lots of > errors due to differing output of the form "page lsn=0/4DAD7E0". You might > turn this into a TAP test and use a regular expression to check the output. > May I ask you to ensure you used v3 of a patch to check? I've

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

2021-03-01 Thread Mark Dilger
> On Feb 9, 2021, at 10:43 AM, Pavel Borisov wrote: > > вт, 9 февр. 2021 г. в 01:46, Mark Dilger : > > > > On Feb 8, 2021, at 2:46 AM, Pavel Borisov wrote: > > > > 0002 - is a temporary hack for testing. It will allow inserting duplicates > > in a table even if an index with the exact

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

2021-02-09 Thread Zhihong Yu
Hi, Minor comment: + if (errflag == true) + ereport(ERROR, I think 'if (errflag)' should suffice. Cheers On Tue, Feb 9, 2021 at 10:44 AM Pavel Borisov wrote: > вт, 9 февр. 2021 г. в 01:46, Mark Dilger : > >> >> >> > On Feb 8, 2021, at 2:46 AM, Pavel Borisov >> wrote: >> > >> > 0002

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

2021-02-09 Thread Pavel Borisov
To make tests stable I also removed lsn output under warning level. PFA v3 of a patch -- Best regards, Pavel Borisov Postgres Professional: http://postgrespro.com v3-0001-Make-amcheck-checking-UNIQUE-constraint-for-btree.patch Description: Binary data

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

2021-02-09 Thread Pavel Borisov
вт, 9 февр. 2021 г. в 01:46, Mark Dilger : > > > > On Feb 8, 2021, at 2:46 AM, Pavel Borisov > wrote: > > > > 0002 - is a temporary hack for testing. It will allow inserting > duplicates in a table even if an index with the exact name "idx" has a > unique constraint (generally it is prohibited

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

2021-02-08 Thread Mark Dilger
> On Feb 8, 2021, at 2:46 AM, Pavel Borisov wrote: > > 0002 - is a temporary hack for testing. It will allow inserting duplicates in > a table even if an index with the exact name "idx" has a unique constraint > (generally it is prohibited to insert). Then a new amcheck will tell us about

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

2021-02-08 Thread Pavel Borisov
On Mon, 8 Feb 2021, 14:46 Pavel Borisov Hi, hackers! > > It seems that if btree index with a unique constraint is corrupted by > duplicates, amcheck now can not catch this. Reindex becomes impossible as > it throws an error but otherwise the index will let the user know that it > is corrupted,

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

2021-02-08 Thread Pavel Borisov
Hi, hackers! It seems that if btree index with a unique constraint is corrupted by duplicates, amcheck now can not catch this. Reindex becomes impossible as it throws an error but otherwise the index will let the user know that it is corrupted, and amcheck will tell that the index is clean. So