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

2013-01-08 Thread Robert Haas
On Tue, Jan 8, 2013 at 7:57 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> I was thinking more about a sprintf()-type function that only
>> understands a handful of escapes, but adds the additional and novel
>> escapes %I (quote as identifier) and %L (quote as literal).  I think
>> that would allow a great deal of code simplification, and it'd be more
>> efficient, too.
>
> Seems like a great idea.  Are you offering to code it?

Not imminently.

> Note that this wouldn't entirely fix the fmtId problem, as not all the
> uses of fmtId are directly in sprintf calls.  Still, it might get rid of
> most of the places where it'd be painful to avoid a memory leak with
> a strdup'ing version of fmtId.

Yeah, I didn't think about that.  Might be worth a look to see how
comprehensively it would solve the problem.  But I'll have to leave
that for another day.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Tom Lane
Robert Haas  writes:
> I was thinking more about a sprintf()-type function that only
> understands a handful of escapes, but adds the additional and novel
> escapes %I (quote as identifier) and %L (quote as literal).  I think
> that would allow a great deal of code simplification, and it'd be more
> efficient, too.

Seems like a great idea.  Are you offering to code it?

Note that this wouldn't entirely fix the fmtId problem, as not all the
uses of fmtId are directly in sprintf calls.  Still, it might get rid of
most of the places where it'd be painful to avoid a memory leak with
a strdup'ing version of fmtId.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Robert Haas
On Tue, Jan 8, 2013 at 6:23 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> And functions that return static buffers are evil incarnate.  I've
>> spent way too much of my life dealing with the supreme idiocy that is
>> fmtId().  If someone ever finds a way to make that go away, I will buy
>> them a beverage of their choice at the next conference we're both at.
>
> Yeah, that was exactly the case that was top-of-mind when I was
> complaining about static return buffers upthread.
>
> It's not hard to make the ugliness go away: just let it strdup its
> return value.  The problem is that in the vast majority of usages it
> wouldn't be convenient to free the result, so we'd have a good deal
> of memory leakage.  What might be interesting is to instrument it to
> see how much (adding a counter to the function ought to be easy enough)
> and then find out whether it's an amount we still care about in 2013.
> Frankly, pg_dump is a memory hog already - a few more identifier-sized
> strings laying about might not matter anymore.
>
> (Wanders away wondering how many relpathbackend callers bother to free
> its result, and whether that matters either ...)

I was thinking more about a sprintf()-type function that only
understands a handful of escapes, but adds the additional and novel
escapes %I (quote as identifier) and %L (quote as literal).  I think
that would allow a great deal of code simplification, and it'd be more
efficient, too.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Tom Lane
Robert Haas  writes:
> And functions that return static buffers are evil incarnate.  I've
> spent way too much of my life dealing with the supreme idiocy that is
> fmtId().  If someone ever finds a way to make that go away, I will buy
> them a beverage of their choice at the next conference we're both at.

Yeah, that was exactly the case that was top-of-mind when I was
complaining about static return buffers upthread.

It's not hard to make the ugliness go away: just let it strdup its
return value.  The problem is that in the vast majority of usages it
wouldn't be convenient to free the result, so we'd have a good deal
of memory leakage.  What might be interesting is to instrument it to
see how much (adding a counter to the function ought to be easy enough)
and then find out whether it's an amount we still care about in 2013.
Frankly, pg_dump is a memory hog already - a few more identifier-sized
strings laying about might not matter anymore.

(Wanders away wondering how many relpathbackend callers bother to free
its result, and whether that matters either ...)

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Andres Freund
On 2013-01-08 17:28:33 -0500, Robert Haas wrote:
> On Tue, Jan 8, 2013 at 3:00 PM, Andres Freund  wrote:
> > Uhm, we don't have & need palloc support and I don't think
> > relpathbackend() is a good justification for adding it.
>
> FWIW, I'm with Tom on this one.  Any meaningful code sharing is going
> to need that, so we might as well bite the bullet.

Yes, if we set the scope bigger than xlogreader I aggree. If its
xlogreader itself I don't. But as I happen to think we should share more
code...
Will prepare a patch.

I wonder whether it would be acceptable to make palloc() an actual
function instead of

#define palloc(sz)  MemoryContextAlloc(CurrentMemoryContext, (sz))

so we don't have to expose CurrentMemoryContext?

Alternatively we can "just" move the whole of utils/mmgr/* to port, but
that would imply an elog/ereport wrapper...

> And functions that return static buffers are evil incarnate.  I've
> spent way too much of my life dealing with the supreme idiocy that is
> fmtId().  If someone ever finds a way to make that go away, I will buy
> them a beverage of their choice at the next conference we're both at.

Imo it depends on the circumstances and number of possible callers, but
anyway, it seems to be already decided that my suggestion isn't the way
to go.

Greetings,

Andres Freund

--
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Alvaro Herrera
Robert Haas escribió:

> And functions that return static buffers are evil incarnate.  I've
> spent way too much of my life dealing with the supreme idiocy that is
> fmtId().

+1

> If someone ever finds a way to make that go away, I will buy
> them a beverage of their choice at the next conference we're both at.

+1

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Robert Haas
On Tue, Jan 8, 2013 at 3:00 PM, Andres Freund  wrote:
> Uhm, we don't have & need palloc support and I don't think
> relpathbackend() is a good justification for adding it.

FWIW, I'm with Tom on this one.  Any meaningful code sharing is going
to need that, so we might as well bite the bullet.

And functions that return static buffers are evil incarnate.  I've
spent way too much of my life dealing with the supreme idiocy that is
fmtId().  If someone ever finds a way to make that go away, I will buy
them a beverage of their choice at the next conference we're both at.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Andres Freund
On 2013-01-08 15:45:07 -0500, Tom Lane wrote:
> Andres Freund  writes:
> > 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)?
> 
> I don't see any need for memory pools, at least not for frontend
> applications of the currently envisioned levels of complexity.  I concur
> with Alvaro's suggestion about just #define'ing them to malloc/free ---
> or maybe better, pg_malloc/free so that we can have a failure-checking
> wrapper.

Unless we want to introduce those into common headers, its more complex
than #define's, you actually need to provide at least
palloc/pfree/CurrentMemoryContext symbols.

Still seems like a shame to do that for one lonely pfree() (+ something
an eventual own implementation of relpathbackend().

> Not sure how we ought to handle elog, but maybe we can put off that bit
> of design until we have a more concrete use-case.

Agreed.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Heikki Linnakangas

On 08.01.2013 23:00, Andres Freund wrote:

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.


Which emulation needs are you missing in the desc routines besides
relpathbackend() and timestamptz_to_str()? pfree() is "just" needed
because its used to free the result of relpathbackend(). No own pallocs,
no ereport ...


Nothing besides those, those are the stuff I was referring to.

- Heikki


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Andres Freund
On 2013-01-08 22:47:43 +0200, Heikki Linnakangas wrote:
> On 08.01.2013 22:39, Andres Freund wrote:
> >On 2013-01-08 15:27:23 -0500, Tom Lane wrote:
> >>Andres Freund  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.

Which emulation needs are you missing in the desc routines besides
relpathbackend() and timestamptz_to_str()? pfree() is "just" needed
because its used to free the result of relpathbackend(). No own pallocs,
no ereport ...

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Heikki Linnakangas

On 08.01.2013 22:39, Andres Freund wrote:

On 2013-01-08 15:27:23 -0500, Tom Lane wrote:

Andres Freund  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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Tom Lane
Andres Freund  writes:
> 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)?

I don't see any need for memory pools, at least not for frontend
applications of the currently envisioned levels of complexity.  I concur
with Alvaro's suggestion about just #define'ing them to malloc/free ---
or maybe better, pg_malloc/free so that we can have a failure-checking
wrapper.

Not sure how we ought to handle elog, but maybe we can put off that bit
of design until we have a more concrete use-case.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Andres Freund
On 2013-01-08 15:27:23 -0500, Tom Lane wrote:
> Andres Freund  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)?

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Alvaro Herrera
Tom Lane wrote:
> Andres Freund  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.

As far as this patch is concerned, I think it's sufficient to do

#define palloc(x) malloc(x)
#define pfree(x) free(x)

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Tom Lane
Andres Freund  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.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Andres Freund
On 2013-01-08 14:53:29 -0500, Tom Lane wrote:
> Andres Freund  writes:
> > On 2013-01-08 14:28:14 -0500, Tom Lane wrote:
> >> I'm 100% unimpressed with making relpathbackend return a pointer to a
> >> static buffer.  Who's to say whether that won't create bugs due to
> >> overlapping usages?
>
> > I say it ;). I've gone through all callers and checked. Not that that
> > guarantees anything, but ...
>
> Even if you've proven it safe today, it seems unnecessarily fragile.
> Just about any other place we've used a static result buffer, we've
> regretted it, unless the use cases were *very* narrow.

Hm, relpathbackend seems pretty narrow to me.

Funny, we both argued the other way round than we are now when talking
about the sprintf(..., "%X/%X", (uint32)(recptr >> 32), (uint32)recptr)
thingy ;)

> > The reason a static buffer is better is that some of the *desc routines
> > use relpathbackend() and pfree() the result. That would require
> > providing a stub pfree() in xlogdump which seems to be exceedingly ugly.
>
> Why?  If we have palloc support how hard can it be to map pfree to free?
> And why wouldn't we want to?  I can hardly imagine providing only palloc
> and not pfree support.

Uhm, we don't have & need palloc support and I don't think
relpathbackend() is a good justification for adding it.

Greetings,

Andres Freund

--
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Tom Lane
Andres Freund  writes:
> On 2013-01-08 14:28:14 -0500, Tom Lane wrote:
>> I'm 100% unimpressed with making relpathbackend return a pointer to a
>> static buffer.  Who's to say whether that won't create bugs due to
>> overlapping usages?

> I say it ;). I've gone through all callers and checked. Not that that
> guarantees anything, but ...

Even if you've proven it safe today, it seems unnecessarily fragile.
Just about any other place we've used a static result buffer, we've
regretted it, unless the use cases were *very* narrow.

> The reason a static buffer is better is that some of the *desc routines
> use relpathbackend() and pfree() the result. That would require
> providing a stub pfree() in xlogdump which seems to be exceedingly ugly.

Why?  If we have palloc support how hard can it be to map pfree to free?
And why wouldn't we want to?  I can hardly imagine providing only palloc
and not pfree support.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Andres Freund
On 2013-01-08 14:28:14 -0500, Tom Lane wrote:
> Andres Freund  writes:
> maxpg> From: Andres Freund 
> > relpathbackend() (via some of its wrappers) is used in *_desc routines 
> > which we
> > want to be useable without a backend environment arround.
>
> I'm 100% unimpressed with making relpathbackend return a pointer to a
> static buffer.  Who's to say whether that won't create bugs due to
> overlapping usages?

I say it ;). I've gone through all callers and checked. Not that that
guarantees anything, but ...

The reason a static buffer is better is that some of the *desc routines
use relpathbackend() and pfree() the result. That would require
providing a stub pfree() in xlogdump which seems to be exceedingly ugly.

> > Change signature to return a 'const char *' to make misuse easier to
> > detect.
>
> That seems to create way more churn than is necessary, and it's wrong
> anyway if the result is palloc'd.

It causes warnings in potential external users that pfree() the result
of relpathbackend which seems helpful. Obviously only makes sense if
relpathbackend() returns a pointer into a static buffer...

Greetings,

Andres Freund

--
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Tom Lane
Andres Freund  writes:
maxpg> From: Andres Freund 
> relpathbackend() (via some of its wrappers) is used in *_desc routines which 
> we
> want to be useable without a backend environment arround.

I'm 100% unimpressed with making relpathbackend return a pointer to a
static buffer.  Who's to say whether that won't create bugs due to
overlapping usages?

> Change signature to return a 'const char *' to make misuse easier to
> detect.

That seems to create way more churn than is necessary, and it's wrong
anyway if the result is palloc'd.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2013-01-08 Thread Andres Freund
From: Andres Freund 

relpathbackend() (via some of its wrappers) is used in *_desc routines which we
want to be useable without a backend environment arround.

Change signature to return a 'const char *' to make misuse easier to
detect. That necessicates also changing the 'FileName' typedef to 'const char
*' which seems to be a good idea anyway.
---
 src/backend/access/rmgrdesc/smgrdesc.c |  6 ++---
 src/backend/access/rmgrdesc/xactdesc.c |  6 ++---
 src/backend/access/transam/xlogutils.c |  9 +++
 src/backend/catalog/catalog.c  | 49 +++---
 src/backend/storage/buffer/bufmgr.c| 12 +++--
 src/backend/storage/file/fd.c  |  6 ++---
 src/backend/storage/smgr/md.c  | 23 +---
 src/backend/utils/adt/dbsize.c |  4 +--
 src/include/catalog/catalog.h  |  2 +-
 src/include/storage/fd.h   |  9 +++
 10 files changed, 42 insertions(+), 84 deletions(-)

diff --git a/src/backend/access/rmgrdesc/smgrdesc.c 
b/src/backend/access/rmgrdesc/smgrdesc.c
index bcabf89..490c8c7 100644
--- a/src/backend/access/rmgrdesc/smgrdesc.c
+++ b/src/backend/access/rmgrdesc/smgrdesc.c
@@ -26,19 +26,17 @@ smgr_desc(StringInfo buf, uint8 xl_info, char *rec)
if (info == XLOG_SMGR_CREATE)
{
xl_smgr_create *xlrec = (xl_smgr_create *) rec;
-   char   *path = relpathperm(xlrec->rnode, xlrec->forkNum);
+   const char *path = relpathperm(xlrec->rnode, xlrec->forkNum);
 
appendStringInfo(buf, "file create: %s", path);
-   pfree(path);
}
else if (info == XLOG_SMGR_TRUNCATE)
{
xl_smgr_truncate *xlrec = (xl_smgr_truncate *) rec;
-   char   *path = relpathperm(xlrec->rnode, MAIN_FORKNUM);
+   const char *path = relpathperm(xlrec->rnode, MAIN_FORKNUM);
 
appendStringInfo(buf, "file truncate: %s to %u blocks", path,
 xlrec->blkno);
-   pfree(path);
}
else
appendStringInfo(buf, "UNKNOWN");
diff --git a/src/backend/access/rmgrdesc/xactdesc.c 
b/src/backend/access/rmgrdesc/xactdesc.c
index 2471279..b86a53e 100644
--- a/src/backend/access/rmgrdesc/xactdesc.c
+++ b/src/backend/access/rmgrdesc/xactdesc.c
@@ -35,10 +35,9 @@ xact_desc_commit(StringInfo buf, xl_xact_commit *xlrec)
appendStringInfo(buf, "; rels:");
for (i = 0; i < xlrec->nrels; i++)
{
-   char   *path = relpathperm(xlrec->xnodes[i], 
MAIN_FORKNUM);
+   const char *path = relpathperm(xlrec->xnodes[i], 
MAIN_FORKNUM);
 
appendStringInfo(buf, " %s", path);
-   pfree(path);
}
}
if (xlrec->nsubxacts > 0)
@@ -105,10 +104,9 @@ xact_desc_abort(StringInfo buf, xl_xact_abort *xlrec)
appendStringInfo(buf, "; rels:");
for (i = 0; i < xlrec->nrels; i++)
{
-   char   *path = relpathperm(xlrec->xnodes[i], 
MAIN_FORKNUM);
+   const char *path = relpathperm(xlrec->xnodes[i], 
MAIN_FORKNUM);
 
appendStringInfo(buf, " %s", path);
-   pfree(path);
}
}
if (xlrec->nsubxacts > 0)
diff --git a/src/backend/access/transam/xlogutils.c 
b/src/backend/access/transam/xlogutils.c
index f9a6e62..8266f3c 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -57,7 +57,7 @@ static void
 report_invalid_page(int elevel, RelFileNode node, ForkNumber forkno,
BlockNumber blkno, bool present)
 {
-   char   *path = relpathperm(node, forkno);
+   const char *path = relpathperm(node, forkno);
 
if (present)
elog(elevel, "page %u of relation %s is uninitialized",
@@ -65,7 +65,6 @@ report_invalid_page(int elevel, RelFileNode node, ForkNumber 
forkno,
else
elog(elevel, "page %u of relation %s does not exist",
 blkno, path);
-   pfree(path);
 }
 
 /* Log a reference to an invalid page */
@@ -153,11 +152,10 @@ forget_invalid_pages(RelFileNode node, ForkNumber forkno, 
BlockNumber minblkno)
{
if (log_min_messages <= DEBUG2 || client_min_messages 
<= DEBUG2)
{
-   char   *path = 
relpathperm(hentry->key.node, forkno);
+   const char *path = 
relpathperm(hentry->key.node, forkno);
 
elog(DEBUG2, "page %u of relation %s has been 
dropped",
 hentry->key.blkno, path);
-   pfree(path);
}
 
if (hash_search(invalid_page_tab,