Re: confusing message in check_tuple

On 18.06.25 11:46, Bertrand Drouvot wrote:
> 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 of attributes 2047 exceeds maximum expected for table 3)
> "
> 
> So that your proposal makes sense to me.

I have committed a fix for this.