Re: Assertion failure in smgr.c when using pg_prewarm with partitioned tables
Fujii Masao <masao.fujii@oss.nttdata.com>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
Cc: Fujii Masao <masao.fujii@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>,
Richard Guo <guofenglinux@gmail.com>,
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-05-29T16:44:25Z
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
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
amcheck: Improve error message for partitioned index target.
- 88a658a42e1b 19 (unreleased) landed
-
Fix assertion failure in pg_prewarm() on objects without storage.
- b64c585fde77 17.6 landed
- 3c4d7557e03b 18.0 landed
-
Prevent assertion failure in contrib/pg_freespacemap.
- 4623d71443de 18.0 cited
-
Don't try to open visibilitymap when analyzing a foreign table
- 049ef3398d05 17.0 cited