Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index.
От | Mark Dilger |
---|---|
Тема | Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index. |
Дата | |
Msg-id | D0DF2FEB-0060-488A-95B2-E102088DB7C4@enterprisedb.com обсуждение исходный текст |
Ответ на | Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index. (Pavel Borisov <pashkin.elfe@gmail.com>) |
Список | pgsql-hackers |
> On Dec 20, 2021, at 7:37 AM, Pavel Borisov <pashkin.elfe@gmail.com> 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 "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 amcheck version check. The implementation of prepare_btree_command() in pg_amcheck.c should be kept compatible with older versions of amcheck,because it connects to remote servers and you can't know in advance that the remote servers are as up-to-date asthe machine where pg_amcheck is installed. I'm thinking specifically about this change: @@ -871,7 +877,8 @@ prepare_btree_command(PQExpBuffer sql, RelationInfo *rel, PGconn *conn) if (opts.parent_check) appendPQExpBuffer(sql, "SELECT %s.bt_index_parent_check(" - "index := c.oid, heapallindexed := %s, rootdescend := %s)" + "index := c.oid, heapallindexed := %s, rootdescend := %s, " + "checkunique := %s)" "\nFROM pg_catalog.pg_class c, pg_catalog.pg_index i " "WHERE c.oid = %u " "AND c.oid = i.indexrelid " If the user calls pg_amcheck with --checkunique, and one or more remote servers have an amcheck version < 1.4, at a minimumyou'll need to avoid calling bt_index_parent_check with that parameter, and probably also you'll either need to raisea warning or perhaps an error telling the user that such a check cannot be performed. You've forgotten to include contrib/amcheck/amcheck--1.3--1.4.sql in the v5 patch, resulting in a failed install: /usr/bin/install -c -m 644 ./amcheck--1.3--1.4.sql ./amcheck--1.2--1.3.sql ./amcheck--1.1--1.2.sql ./amcheck--1.0--1.1.sql./amcheck--1.0.sql '/Users/mark.dilger/hydra/unique_review.5/tmp_install/Users/mark.dilger/pgtest/test_install/share/postgresql/extension/' install: ./amcheck--1.3--1.4.sql: No such file or directory make[2]: *** [install] Error 71 make[1]: *** [checkprep] Error 2 Using the one from the v4 patch fixes the problem. Please include this file in v6, to simplify the review process. The changes to t/005_opclass_damage.pl look ok. The creation of a new table for the new test seems unnecessary, but onlyproblematic in that it makes the test slightly longer to read. I recommend changing the test to use the same table thatthe prior test uses, but that is just a recommendation, not a requirement. You should add coverage for --checkunique to t/003_check.pl. You should add coverage for multiple PostgreSQL::Test::Cluster instances running differing versions of amcheck, perhaps someon version 1.3 and some on version 1.4. Then test that the --checkunique option works adequately. I have not reviewed the changes to verify_nbtree.c. I'll leave that to Peter. — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
В списке pgsql-hackers по дате отправления: