Re: GetRelationPath() vs critical sections - Mailing list pgsql-hackers

From Noah Misch
Subject Re: GetRelationPath() vs critical sections
Date
Msg-id 20250220190315.ad.nmisch@google.com
Whole thread Raw
In response to Re: GetRelationPath() vs critical sections  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
On Thu, Feb 20, 2025 at 12:40:57PM -0500, Andres Freund wrote:
> On 2025-02-20 14:00:10 +1300, Thomas Munro wrote:
> > On Wed, Feb 19, 2025 at 3:35 PM Andres Freund <andres@anarazel.de> wrote:
> > > After thinking about this for an embarassingly long time, I think there's
> > > actually a considerably better answer for a case like this: A function that
> > > returns a fixed-length string by value:
> > >
> > > - Compilers can fairly easily warn about on-stack values that goes out of
> > >   scope
> > >
> > > - Because we don't need to free the memory anymore, some code that that
> > >   previously needed to explicitly free the memory doesn't need to anymore
> > >   (c.f. AbortBufferIO()).
> > >
> > > - The max lenght isn't that long, so it's actually reasonably efficient,
> > >   likely commonly cheaper than psprintf.
> > 
> > I like it!

Works for me.

> Unfortunately I had to exclude "relpath" as there are just too many
> independent hits, due to the python function of the same name. For
> relpathperm(), relpathbackend(), GetRelationPath() there looks to be just
> fincore.

PGXN has few hits, and some of these are false positives or otherwise
irrelevant:

$ grep -re '[^.]\(relpath[a-z]*\|GetRelationPath\)(' | sed 's/-[^:]*/:/'|sort -u
db2_fdw::extern char *GetRelationPath(Oid dbNode, Oid spcNode, Oid relNode,
jsoncdc::    pub fn GetRelationPath(dbNode: Oid, spcNode: Oid, relNode: Oid,
openbarter::        path = relpath(frame.f_code.co_filename, refdir)  # relative to refdir
pg_bulkload::#define relpath(rnode, forknum)            relpath((rnode))
pg_bulkload::   fname = relpath(bknode, MAIN_FORKNUM);
pg_bulkload::   fname = relpath(rnode, MAIN_FORKNUM);
pg_repack::#define relpath(rnode, forknum)              relpath((rnode))
plv8::  def _to_relpath(self, abspath, _):
plv8::  def _to_relpath(self, abspath, test_root):
plv8::          yield self._to_relpath(abspath, test_root)
tblsize_nolock::        relationpath = relpath(*rfn);

> Which makes me think it's not worth having a backward compatible interface?

Agreed.  Even if 100% of those matches had to change, that's below standard
level of breakage for a major release.



pgsql-hackers by date:

Previous
From: Pavel Stehule
Date:
Subject: broken munhttps://github.com/munin-monitoring/contrib/issues/1483in plugins for PostgreSQL 17
Next
From: Tom Lane
Date:
Subject: Re: GetRelationPath() vs critical sections