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

> On Dec 22, 2021, at 12:01 AM, Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com> 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