From: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> |
---|---|
To: | Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com> |
Cc: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Richard Guo <guofenglinux(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Assertion failure in smgr.c when using pg_prewarm with partitioned tables |
Date: | 2025-05-29 16:44:25 |
Message-ID: | 23d5aea0-be92-4837-a5ee-1bdf0ebc49c5@oss.nttdata.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2025/05/29 11:46, Masahiro Ikeda wrote:
> Thanks for your feedback. I've attached the updated patches.
I've pushed the 0001 patch. Thanks!
> 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.
Thanks for considering this. The proposed messages look good to me.
>> 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 may have been mistaken earlier. Based on the comment in SearchSysCache(),
the tuple returned by SearchSysCache1() is valid until the end of the transaction.
Since index_checkable() raises an error and ends the transaction immediately
after calling SearchSysCache1(), it seems safe to skip ReleaseSysCache()
in this case. Using get_am_name() instead seems simpler, though.
Thought?
>> 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.
+1.
Should we commit patch 0003 before 0002? Also, should we consider back-patching it?
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2025-05-29 17:02:39 | Re: Fixing memory leaks in postgres_fdw |
Previous Message | Bruce Momjian | 2025-05-29 16:41:17 | Re: PG 18 release notes draft committed |