Re: Assertion failure in smgr.c when using pg_prewarm with partitioned tables

Thanks for your feedback. I've attached the updated patches.

On 2025-05-28 10:10, Fujii Masao wrote:
> On 2025/05/26 16:55, ikedamsh wrote:
>> 2025/05/21 12:54 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
>>     Regarding the 0002 patch:
>> 
>>     - errdetail("Relation \"%s\" is a %s index.",
>>     -    RelationGetRelationName(rel), NameStr(((Form_pg_am)
>>     GETSTRUCT(amtuprel))->amname))));
>>     + errdetail("Relation \"%s\" is a %s %sindex.",
>>     +    RelationGetRelationName(rel), NameStr(((Form_pg_am)
>>     GETSTRUCT(amtuprel))->amname),
>>     +    (rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX) ?
>>     "partitioned " : "")));
>> 
>>     Instead of manually building the message, how about using
>>     errdetail_relkind_not_supported()?
>>     It would be more consistent with similar error reporting 
>> elsewhere.
>> 
>> I was thinking of using errdetail_relkind_not_supported(),
>> but I’m reconsidering building the message manually
>> since the AM name seems to be important for the error.
>> What do you think?
> 
> Understood.
> I was trying to better understand the error message, as I found
> the following is still a bit confusing for users. However, I don't
> have a better suggestion at the moment, so I'm okay with
> the proposed change.
> 
> ERROR:  expected "btree" index as targets for verification
> DETAIL:  Relation "pgbench_accounts_pkey" is a btree partitioned

What do you think about adding new error messages specifically for 
partitioned
indexes?

If the target is a partitioned index, the error messages would be:

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

If the target is an index, but its access method is not supported, the 
error
messages would be:

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

> This is not directly relation to your proposal, but while reading
> the index_checkable() function, I noticed that ReleaseSysCache()
> is not called after SearchSysCache1(). Shouldn't we call
> ReleaseSysCache() here? Alternatively, we could use get_am_name()
> instead of SearchSysCache1(), which might be simpler.

Agreed.

> I also observed that the error code ERRCODE_FEATURE_NOT_SUPPORTED
> is used when the relation is not the expected type in 
> index_checkable().
> However, based on similar cases (e.g., pgstattuple), it seems that
> ERRCODE_WRONG_OBJECT_TYPE might be more appropriate in this situation.
> Thought?

Agreed. I also change the error code to 
ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE
when the index is not valid.

Regards,
-- 
Masahiro Ikeda
NTT DATA Japan Corporation