Re: Attempt to consolidate reading of XLOG page

On Mon, May 6, 2019 at 2:21 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> On 2019-May-06, Robert Haas wrote:
> > On Thu, May 2, 2019 at 12:18 PM Antonin Houska <ah@cybertec.at> wrote:
> > > The next version of the patch is attached.
> >
> > I don't think any of this looks acceptable:
>
> I agree.  I inteded to suggest upthread to pass an additional argument
> to XLogRead, which is a function that takes a message string and
> SQLSTATE; in backend, the function does errstart / errstate / errmsg /
> errfinish, and in frontend programs it does pg_log_fatal (and ignores
> sqlstate).  The message must be sprintf'ed and translated by XLogRead.
> (xlogreader.c could itself provide a default error reporting callback,
> at least for frontend, to avoid repeating the code).  That way, if a
> different frontend program wants to do something different, it's fairly
> easy to pass a different function pointer.

It seems to me that it's better to unwind the stack i.e. have the
function return the error information to the caller and let the caller
do as it likes.  The other thread to which Horiguchi-san referred
earlier in this thread seems to me to have basically concluded that
the XLogPageReadCB callback to XLogReaderAllocate is a pain to use
because it doesn't unwind the stack, and work is under way over there
to get rid of that callback for just that reason.  Adding a new
callback for error-reporting would just be creating a new instance of
the same issue.

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