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

From: Aleksander Alekseev <aleksander(at)timescale(dot)com>
To: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com>, Maxim Orlov <orlovmg(at)gmail(dot)com>, lubennikovaav(at)gmail(dot)com, Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, Peter Geoghegan <pg(at)bowt(dot)ie>
Subject: Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index.
Date: 2022-07-20 14:15:34
Message-ID: CAJ7c6TP+Optg9TMZJd9+ic9R_c8RLU8DcUCyCoDnZZobb_1wQg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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 Content-Type Size
v14-0001-Add-option-for-amcheck-and-pg_amcheck-to-check-u.patch application/octet-stream 43.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-07-20 14:15:55 Re: Re: pgsql: Default to hidden visibility for extension libraries where possi
Previous Message Andres Freund 2022-07-20 14:12:43 Re: pgsql: Default to hidden visibility for extension libraries where possi