Re: confusing message in check_tuple

Hi,

On Tue, Jun 17, 2025 at 07:21:13AM +0200, Peter Eisentraut wrote:
> On 12.06.25 08:26, jian he wrote:
> > in contrib/amcheck/verify_heapam.c, check_tuple
> >          report_corruption(ctx,
> >                            psprintf("number of attributes %u exceeds
> > maximum expected for table %u",
> >                                     ctx->natts,
> >                                     RelationGetDescr(ctx->rel)->natts));
> 
> Agreed this is misleading.
> 
> > i think it should be
> >          report_corruption(ctx,
> >                            psprintf("number of attributes %u exceeds
> > maximum expected for table %u",
> >                                     ctx->natts,
> >                                     RelationGetRelid(ctx->rel)));
> > 
> > or we can rephrase it another way, also mentioning
> > ``RelationGetDescr(ctx->rel)->natts``.
> 
> I think they did want to mention RelationGetDescr(ctx->rel)->natts.

+1, I think that we usually want to be able to compare actual vs expected.

> How
> about
> 
> "number of attributes %u exceeds maximum expected for table (%u)"

I thought about adding the table name in the message but it looks like it's already
there:

"
Expected corruption message output stdout /(?^:(?^ms:heap table "postgres\.public\.test", block 0, offset 9:\s+)number
ofattributes 2047 exceeds maximum expected for table 3)
 
"

So that your proposal makes sense to me.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com