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

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-07-04 11:32:37
Message-ID: 1a3170ba-a2d2-4519-be04-6b620f698be3@oss.nttdata.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2025/06/02 16:32, Masahiro Ikeda wrote:
> OK, I think v5-0002 should be back-patched, since using incorrect error codes is essentially a bug.

Thanks for updating the patches!

While reviewing the code:

amtup = SearchSysCache1(AMOID, ObjectIdGetDatum(am_id));
amtuprel = SearchSysCache1(AMOID, ObjectIdGetDatum(rel->rd_rel->relam));
ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("expected \"%s\" index as targets for verification", NameStr(((Form_pg_am) GETSTRUCT(amtup))->amname)),
errdetail("Relation \"%s\" is a %s index.",
RelationGetRelationName(rel), NameStr(((Form_pg_am) GETSTRUCT(amtuprel))->amname))));

I initially thought switching to ERRCODE_WRONG_OBJECT_TYPE made sense,
but after checking similar cases, I'm reconsidering. For instance,
pgstat_relation() in pgstattuple.c uses ERRCODE_FEATURE_NOT_SUPPORTED
when the access method is unexpected. That suggests using
FEATURE_NOT_SUPPORTED here may not be incorrect.

if (!rel->rd_index->indisvalid)
ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("cannot check index \"%s\"",
RelationGetRelationName(rel)),
errdetail("Index is not valid.")));

Other parts of the codebase (including pgstattuple) use
ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE when rejecting invalid index.
So, changing to this error code still seems reasonable and more
consistent overall.

Given that, I'd like to merge just this error code change from
the 0002 patch into 0003, and apply the combined patch to
the master branch only. Although I briefly considered backpatching
the change of error code, it could be seen as a behavioral change,
and the existing error code doesn't cause any real problem in older
branches. So it's probably better to leave it as-is there.

The updated patch is attached. It also changes index_checkable()
from extern to static, since it's only used within verify_common.c.

Thoughts?

Regards,

--
Fujii Masao
NTT DATA Japan Corporation

Attachment Content-Type Size
v6-0001-amcheck-Improve-error-message-for-partitioned-ind.patch text/plain 5.9 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Nikita Malakhov 2025-07-04 11:38:34 Re: Support for 8-byte TOAST values (aka the TOAST infinite loop problem)
Previous Message Zhijie Hou (Fujitsu) 2025-07-04 11:18:38 RE: Conflict detection for update_deleted in logical replication