Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index. - Mailing list pgsql-hackers

From Aleksander Alekseev
Subject Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index.
Date
Msg-id CAJ7c6TP+Optg9TMZJd9+ic9R_c8RLU8DcUCyCoDnZZobb_1wQg@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index.  (Pavel Borisov <pashkin.elfe@gmail.com>)
Responses Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index.
List pgsql-hackers
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 bt_index_parent_check('bttest_b_idx', true, false, true);
``

1. This "true, false, true" sequence is difficult to read. I suggest
we use named arguments here.

2. I believe there are some minor issues with the comments. E.g. instead of:

- First key on next page is same
- Make values 768 and 769 looks equal

I would write:

- The first key on the next page is the same
- Make values 768 and 769 look equal

There are many little errors like these.

```
+# Copyright (c) 2021, PostgreSQL Global Development Group
```

3. Oh no. The copyright has expired!

```
+      <literal>true</literal>.  When <parameter>checkunique</parameter>
+      is <literal>true</literal> <function>bt_index_check</function> will
```

4. This piece of documentation was copy-pasted between two functions
without change of the function name.

Other than that to me the patch looks in pretty good shape. Here is
v14 where I fixed my own nitpicks, with the permission of Pavel given
offlist.

If no one sees any other defects I'm going to change the status of the
patch to "Ready to Committer" in a short time.

--
Best regards,
Aleksander Alekseev

Attachment

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: pgsql: Default to hidden visibility for extension libraries where possi
Next
From: Tom Lane
Date:
Subject: Re: Re: pgsql: Default to hidden visibility for extension libraries where possi