From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Dilip Kumar <dilipbalaut@gmail.com>
To: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
Cc: Richard Guo <guofenglinux@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Assertion failure in smgr.c when using pg_prewarm with partitioned tables
Date: Fri, 16 May 2025 21:01:24 +0900
Message-ID: <5a494c43-326e-45c3-b391-e95501152bb2@oss.nttdata.com> (raw)
In-Reply-To: <CAFiTN-s_34r+vTWUdnyfAMA4F07QEdPOhAKkRVzwgZeeJPw3Yw@mail.gmail.com>
References: <e082e6027610fd0a4091ae6d033aa117@oss.nttdata.com>
<CAFiTN-tKxhMG0QNO1V3VNsfnhtkS=hrOJNj5Kxs6cGKtujdyhg@mail.gmail.com>
<7fde0acf-2395-44d8-b081-bfa0e10097dc@oss.nttdata.com>
<CAFiTN-vDKGHn76XkL2oTQVf5aCScKqs1GKf_aALGO5u7fuqNwg@mail.gmail.com>
<CAMbWs49T8hFr571c+b0T4GiDwKow5B1HUMMxav7qhUt75WHpKA@mail.gmail.com>
<8829854bbfc8635ddecd0846bb72dfda@oss.nttdata.com>
<CAFiTN-s_34r+vTWUdnyfAMA4F07QEdPOhAKkRVzwgZeeJPw3Yw@mail.gmail.com>
On 2025/05/16 15:33, Dilip Kumar wrote:
> On Fri, May 16, 2025 at 11:51 AM Masahiro Ikeda
> <ikedamsh@oss.nttdata.com> wrote:
>>
>> Thank you for your comments!
>>
>> I updated the patch to use RELKIND_HAS_STORAGE() as done in
>> commit 4623d7144. Please see the v2-0001 patch for the changes.
Thanks for updating the patch!
You added a test for pg_prewarm() with a partitioned table in the TAP tests.
But, wouldn't it be simpler and easier to maintain if we added this
as a regular regression test (i.e., using .sql and .out files) instead of
TAP tests? That should be sufficient for this case?
Also, since the issue was introduced in v17, this patch should be
back-patched to v17, right?
>> On 2025-05-15 19:57, Richard Guo wrote:
>>> +1. FWIW, not long ago we fixed a similar Assert failure in
>>> contrib/pg_freespacemap by verifying RELKIND_HAS_STORAGE() before
>>> trying to access the storage (see 4623d7144). Wondering if there are
>>> other similar issues elsewhere in contrib ...
>>
>> I tested all contrib functions that take regclass arguments (see
>> attached test.sql and test_result.txt). The result shows that only
>> pg_prewarm() can lead to the assertion failure.
>
> Right, even while I was searching, I see this is used in 3 contrib
> modules, amcheck, autoprewarm, and pg_prewarm. amcheck is already
> checking for AM type, and Autoprewarm is identifying the relation by
> block, so there is no chance of getting the relation which do not have
> storage.
>
>>
>> However, I found that amcheck's error messages can be misleading
>> when run on partitioned indexes.
>>
>> For example, on the master branch, amcheck (e.g., bt_index_check
>> function)
>> shows this error:
>>
>> > ERROR: expected "btree" index as targets for verification
>> > DETAIL: Relation "pgbench_accounts_pkey" is a btree index.
>>
>> This message says it expects a "btree" index, yet also states the
>> relation
>> is a "btree" index, which can seem contradictory. The actual issue is
>> that
>> the index is a btree partitioned index, but this detail is missing,
>> causing
>> possible confusion.
>
> Yes, I found the error message confusing during testing as well, so it
> makes sense to improve it.
+1
Regarding the patch, how about also adding a regression test for
amcheck with a partitioned index?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: masao.fujii@oss.nttdata.com, dilipbalaut@gmail.com, ikedamsh@oss.nttdata.com, guofenglinux@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Assertion failure in smgr.c when using pg_prewarm with partitioned tables
In-Reply-To: <5a494c43-326e-45c3-b391-e95501152bb2@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox