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