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

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