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

On 08.01.2013 22:39, Andres Freund wrote:
> On 2013-01-08 15:27:23 -0500, Tom Lane wrote:
>> Andres Freund<andres@2ndquadrant.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