public inbox for [email protected]  

help / color / mirror / Atom feed

From: Mark Dilger <[email protected]>
To: Pavel Borisov <[email protected]>
Cc: David Steele <[email protected]>
Cc: Postgres hackers <[email protected]>
Cc: Peter Geoghegan <[email protected]>
Cc: Maxim Orlov <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index.
Date: Thu, 23 Dec 2021 08:31:29 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALT9ZEEx+3NwspWRx2eRRWOG16SYtKSD+7825jAh1wdJux_EkA@mail.gmail.com>
References: <CALT9ZEHRn5xAM5boga0qnrCmPV52bScEK2QnQ1HmUZDD301JEg@mail.gmail.com>
	<[email protected]>
	<CALT9ZEFYsYPms0SPp4ZxpFCOBQbgE+iopoVKzU8Pni8Y6+RKtQ@mail.gmail.com>
	<[email protected]>
	<CALT9ZEGgKAXDfVaAjBjBf4e6-L_4NVQsJ+7gzHd0JLwNxjXTFA@mail.gmail.com>
	<[email protected]>
	<CALT9ZEFMb96zVafrt7sUPrWYMDQVngaPcXuVqXCfawA2cfWwxw@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CALT9ZEF=1q-mJiHV6E9e0cQmOYur0vtSX387KLZGs5U3me7vxw@mail.gmail.com>
	<[email protected]>
	<CALT9ZEEx+3NwspWRx2eRRWOG16SYtKSD+7825jAh1wdJux_EkA@mail.gmail.com>



> On Dec 22, 2021, at 12:01 AM, Pavel Borisov <[email protected]> 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.

Ok.

+           int         vmaj = 0,
+                       vmin = 0,
+                       vrev = 0;
+           const char *amcheck_version = pstrdup(PQgetvalue(result, 0, 1));
+
+           sscanf(amcheck_version, "%d.%d.%d", &vmaj, &vmin, &vrev);

The pstrdup is unnecessary but harmless.

> - fixed unnecessary drop table in regression

Ok.

> - 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 smoke test that just verifies new functions.

+
+$node->command_checks_all(
+   [
+       @cmd, '-s', 's1', '-i', 't1_btree', '--parent-check',
+       '--checkunique', 'db1'
+   ],
+   2,
+   [$index_missing_relation_fork_re],
+   [$no_output_re],
+   'pg_amcheck smoke test --parent-check');
+
+$node->command_checks_all(
+   [
+       @cmd, '-s', 's1', '-i', 't1_btree', '--heapallindexed',
+       '--rootdescend', '--checkunique',  'db1'
+   ],
+   2,
+   [$index_missing_relation_fork_re],
+   [$no_output_re],
+   'pg_amcheck smoke test --heapallindexed --rootdescend');
+
+$node->command_checks_all(
+   [ @cmd, '--checkunique', '-d', 'db1', '-d', 'db2', '-d', 'db3', '-S', 's*' ],
+   0, [$no_output_re], [$no_output_re],
+   'pg_amcheck excluding all corrupt schemas');
+

You have borrowed the existing tests but forgot to change their names.  (The last line of each check is the test name, such as 'pg_amcheck smoke test --parent-check'.)  Please make each test name unique.

> - added test contrib/amcheck/t/004_verify_nbtree_unique.pl it is more extensive test based on opclass damage which was intended to be main test for amcheck, but which I've forgotten to add to commit in v5.
> 005_opclass_damage.pl test, which you've seen in v5 is largely based on first part of 004_verify_nbtree_unique.pl (with the later calling pg_amcheck, and the former calling bt_index_check(), bt_index_parent_check() )

Ok.

> - added forgotten upgrade script amcheck--1.3--1.4.sql (from v4)

Ok.


—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

view thread (32+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index.
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox