Re: Fix Error Message for allocate_recordbuf() Failure

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Shoaib Lari <slari(at)pivotal(dot)io>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix Error Message for allocate_recordbuf() Failure
Date: 2016-09-20 15:32:47
Message-ID: CA+TgmoagYiUmz0eERRhYPuGwtjV=2zH_1M5redKdPjk++FNkPw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jul 11, 2016 at 12:04 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Sat, Jul 9, 2016 at 2:58 AM, Shoaib Lari <slari(at)pivotal(dot)io> wrote:
>> Besides making the error message more informative, we had to modify
>> allocate_recordbuf() to return the actual number of bytes that were being
>> allocated.
>
> - report_invalid_record(state, "record length %u at %X/%X too long",
> - total_len,
> - (uint32) (RecPtr >> 32), (uint32) RecPtr);
> + report_invalid_record(state,
> + "cannot allocate %u bytes for record
> length %u at %X/%X",
> + newSizeOut, total_len, (uint32) (RecPtr >> 32),
> + (uint32) RecPtr);
>
> It does not look like a good idea to me to complicate the interface of
> allocate_recordbuf just to make more verbose one error message, ...

For what it's worth, I think it's fine. Good error messages are a useful thing.

More generally, I think the whole patch looks good and should be committed.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-09-20 15:40:29 Re: Odd system-column handling in postgres_fdw join pushdown patch
Previous Message Peter Eisentraut 2016-09-20 15:31:53 Re: Logical Replication WIP