amcheck: Improve error message for partitioned index target.

Previously, amcheck could produce misleading error message when
a partitioned index was passed to functions like bt_index_check().
For example, bt_index_check() with a partitioned btree index produced:

    ERROR:  expected "btree" index as targets for verification
    DETAIL:  Relation ... is a btree index.

Reporting "expected btree index as targets" even when the specified
index was a btree was confusing. In this case, the function should fail
since the partitioned index specified is not valid target. This commit
improves the error reporting to better reflect this actual issue. Now,
bt_index_check() with a partitioned index, the error message is:

    ERROR:  expected index as targets for verification
    DETAIL:  This operation is not supported for partitioned indexes.

This commit also applies the following minor changes:

- Simplifies index_checkable() by using get_am_name() to retrieve
   the access method name.

- Changes index_checkable() from extern to static, as it is only used
   in verify_common.c.

- Updates the error code for invalid indexes to
   ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE,
   aligning with usage in similar modules like pgstattuple.

Author: Masahiro Ikeda <[email protected]>
Reviewed-by: Fujii Masao <[email protected]>
Discussion: https://postgr.es/m/8829854bbfc8635ddecd0846bb72dfda@oss.nttdata.com

diff --git a/contrib/amcheck/expected/check_btree.out b/contrib/amcheck/expected/check_btree.out
index c6f4b16c55615b1fc1a3cdc0c94bca55e7c19505..6558f2c5a4ff41d0b7b77ddb7a7f66a751042bfd 100644 (file)
--- a/contrib/amcheck/expected/check_btree.out
+++ b/contrib/amcheck/expected/check_btree.out
@@ -60,6 +60,14 @@ SELECT bt_index_parent_check('bttest_a_brin_idx');
 ERROR:  expected "btree" index as targets for verification
 DETAIL:  Relation "bttest_a_brin_idx" is a brin index.
 ROLLBACK;
+-- verify partitioned indexes are rejected (error)
+BEGIN;
+CREATE TABLE bttest_partitioned (a int, b int) PARTITION BY list (a);
+CREATE INDEX bttest_btree_partitioned_idx ON bttest_partitioned USING btree (b);
+SELECT bt_index_parent_check('bttest_btree_partitioned_idx');
+ERROR:  expected index as targets for verification
+DETAIL:  This operation is not supported for partitioned indexes.
+ROLLBACK;
 -- normal check outside of xact
 SELECT bt_index_check('bttest_a_idx');
  bt_index_check 

diff --git a/contrib/amcheck/sql/check_btree.sql b/contrib/amcheck/sql/check_btree.sql
index 0793dbfeebd821f5617e23fa889155b198801c26..171f7f691ec60545376822e4e9e29cc02f9dcef2 100644 (file)
--- a/contrib/amcheck/sql/check_btree.sql
+++ b/contrib/amcheck/sql/check_btree.sql
@@ -52,6 +52,13 @@ CREATE INDEX bttest_a_brin_idx ON bttest_a USING brin(id);
 SELECT bt_index_parent_check('bttest_a_brin_idx');
 ROLLBACK;
 
+-- verify partitioned indexes are rejected (error)
+BEGIN;
+CREATE TABLE bttest_partitioned (a int, b int) PARTITION BY list (a);
+CREATE INDEX bttest_btree_partitioned_idx ON bttest_partitioned USING btree (b);
+SELECT bt_index_parent_check('bttest_btree_partitioned_idx');
+ROLLBACK;
+
 -- normal check outside of xact
 SELECT bt_index_check('bttest_a_idx');
 -- more expansive tests

diff --git a/contrib/amcheck/verify_common.c b/contrib/amcheck/verify_common.c
index d095e62ce551f364cd2c17070ecf9d5709351f20..a31ce06ed99a335c37374322a14aa637d936de11 100644 (file)
--- a/contrib/amcheck/verify_common.c
+++ b/contrib/amcheck/verify_common.c
@@ -18,11 +18,13 @@
 #include "verify_common.h"
 #include "catalog/index.h"
 #include "catalog/pg_am.h"
+#include "commands/defrem.h"
 #include "commands/tablecmds.h"
 #include "utils/guc.h"
 #include "utils/syscache.h"
 
 static bool amcheck_index_mainfork_expected(Relation rel);
+static bool index_checkable(Relation rel, Oid am_id);
 
 
 /*
@@ -155,23 +157,21 @@ amcheck_lock_relation_and_check(Oid indrelid,
  * callable by non-superusers. If granted, it's useful to be able to check a
  * whole cluster.
  */
-bool
+static bool
 index_checkable(Relation rel, Oid am_id)
 {
-   if (rel->rd_rel->relkind != RELKIND_INDEX ||
-       rel->rd_rel->relam != am_id)
-   {
-       HeapTuple   amtup;
-       HeapTuple   amtuprel;
+   if (rel->rd_rel->relkind != RELKIND_INDEX)
+       ereport(ERROR,
+               (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                errmsg("expected index as targets for verification"),
+                errdetail_relkind_not_supported(rel->rd_rel->relkind)));
 
-       amtup = SearchSysCache1(AMOID, ObjectIdGetDatum(am_id));
-       amtuprel = SearchSysCache1(AMOID, ObjectIdGetDatum(rel->rd_rel->relam));
+   if (rel->rd_rel->relam != am_id)
        ereport(ERROR,
                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                errmsg("expected \"%s\" index as targets for verification", NameStr(((Form_pg_am) GETSTRUCT(amtup))->amname)),
+                errmsg("expected \"%s\" index as targets for verification", get_am_name(am_id)),
                 errdetail("Relation \"%s\" is a %s index.",
-                          RelationGetRelationName(rel), NameStr(((Form_pg_am) GETSTRUCT(amtuprel))->amname))));
-   }
+                          RelationGetRelationName(rel), get_am_name(rel->rd_rel->relam))));
 
    if (RELATION_IS_OTHER_TEMP(rel))
        ereport(ERROR,
@@ -182,7 +182,7 @@ index_checkable(Relation rel, Oid am_id)
 
    if (!rel->rd_index->indisvalid)
        ereport(ERROR,
-               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+               (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                 errmsg("cannot check index \"%s\"",
                        RelationGetRelationName(rel)),
                 errdetail("Index is not valid.")));

diff --git a/contrib/amcheck/verify_common.h b/contrib/amcheck/verify_common.h
index 42ef9c20fe244e647a75f834ce075d60ddc0771a..3f4c57f963d6bbc7858202bdb408d9d70cf15a36 100644 (file)
--- a/contrib/amcheck/verify_common.h
+++ b/contrib/amcheck/verify_common.h
@@ -26,5 +26,3 @@ extern void amcheck_lock_relation_and_check(Oid indrelid,
                                            Oid am_id,
                                            IndexDoCheckCallback check,
                                            LOCKMODE lockmode, void *state);
-
-extern bool index_checkable(Relation rel, Oid am_id);