From e8f158d36bad49966f2843a908a0a270bd441357 Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Fri, 4 Jul 2025 14:14:05 +0900 Subject: [PATCH v6] 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() witha 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 Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/8829854bbfc8635ddecd0846bb72dfda@oss.nttdata.com --- contrib/amcheck/expected/check_btree.out | 8 ++++++++ contrib/amcheck/sql/check_btree.sql | 7 +++++++ contrib/amcheck/verify_common.c | 22 +++++++++++----------- contrib/amcheck/verify_common.h | 2 -- 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/contrib/amcheck/expected/check_btree.out b/contrib/amcheck/expected/check_btree.out index c6f4b16c556..6558f2c5a4f 100644 --- 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 0793dbfeebd..171f7f691ec 100644 --- 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 d095e62ce55..7e9c5eda1bd 100644 --- 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); /* @@ -158,20 +160,18 @@ amcheck_lock_relation_and_check(Oid indrelid, 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 e78adb68808..7e56a2d3c4a 100644 --- a/contrib/amcheck/verify_common.h +++ b/contrib/amcheck/verify_common.h @@ -27,5 +27,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); -- 2.49.0