Re: [PATCH 2/5] Make relpathbackend return a statically result instead of palloc()'ing it

From: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH 2/5] Make relpathbackend return a statically result instead of palloc()'ing it
Date: 2013-01-08 20:47:43
Message-ID: 50EC85EF.2080703@vmware.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 08.01.2013 22:39, Andres Freund wrote:
> On 2013-01-08 15:27:23 -0500, Tom Lane wrote:
>> Andres Freund<andres(at)2ndquadrant(dot)com> writes:
>>> Uhm, we don't have& need palloc support and I don't think
>>> relpathbackend() is a good justification for adding it.
>>
>> I've said from the very beginning of this effort that it would be
>> impossible to share any meaningful amount of code between frontend and
>> backend environments without adding some sort of emulation of
>> palloc/pfree/elog. I think this patch is just making the code uglier
>> and more fragile to put off the inevitable, and that we'd be better
>> served to bite the bullet and do that.
>
> If you think relpathbackend() alone warrants that, yes, I can provide a
> wrapper. Everything else is imo already handled in a sensible and not
> really ugly manner? Imo its not worth the effort *for this alone*.
>
> I already had some elog(), ereport(), whatever emulation but Heikki
> preferred not to have it, so its removed by now.
>
> To what extent do you want palloc et al. emulation? Provide actual pools
> or just make redirect to malloc and provide the required symbols (at the
> very least CurrentMemoryContext)?

Note that the xlogreader facility doesn't need any more emulation. I'm
quite satisfied with that part of the patch now. However, the rmgr desc
routines do, and I'm not very happy with those. Not sure what to do
about it. As you said, we could add enough infrastructure to make them
compile in frontend context, or we could try to make them rely less on
backend functions.

One consideration is that once we have a separate pg_xlogdump utility, I
expect that to be the most visible consumer of the rmgr desc functions.
The backend will of course still use those functions in e.g error
messages, but those don't happen very often. Not sure how that should be
taken into account in this patch, but I thought I'd mention it.

An rmgr desc routine probably shouldn't be calling elog() even in the
backend, because you don't want to throw errors in the contexts where
those routines are called.

- Heikki

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2013-01-08 20:55:24 Re: Re: [PATCH 1/5] Centralize Assert* macros into c.h so its common between backend/frontend
Previous Message Tom Lane 2013-01-08 20:45:07 Re: [PATCH 2/5] Make relpathbackend return a statically result instead of palloc()'ing it