pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Andres Freund <andres@anarazel.de>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: Change relpath() et al to return path by value
Date: Tue, 25 Feb 2025 14:23:30 +0000
Message-ID: <E1tmvqL-0001EL-0K@gemulon.postgresql.org> (raw)

Change relpath() et al to return path by value

For AIO, and also some other recent patches, we need the ability to call
relpath() in a critical section. Until now that was not feasible, as it
allocated memory.

The fact that relpath() allocated memory also made it awkward to use in log
messages because we had to take care to free the memory afterwards. Which we
e.g. didn't do for when zeroing out an invalid buffer.

We discussed other solutions, e.g. filling a pre-allocated buffer that's
passed to relpath(), but they all came with plenty downsides or were larger
projects. The easiest fix seems to be to make relpath() return the path by
value.

To be able to return the path by value we need to determine the maximum length
of a relation path. This patch adds a long #define that computes the exact
maximum, which is verified to be correct in a regression test.

As this change the signature of relpath(), extensions using it will need to
adapt their code. We discussed leaving a backward-compat shim in place, but
decided it's not worth it given the use of relpath() doesn't seem widespread.

Discussion: https://postgr.es/m/xeri5mla4b5syjd5a25nok5iez2kr3bm26j2qn4u7okzof2bmf@kwdh2vf7npra

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/37c87e63f9e1a2d76db54fedcdf91d3895d200a6

Modified Files
--------------
src/backend/access/rmgrdesc/smgrdesc.c          | 10 ++--
src/backend/access/rmgrdesc/xactdesc.c          |  6 +-
src/backend/access/transam/xlogutils.c          | 29 +++-------
src/backend/backup/basebackup_incremental.c     | 10 ++--
src/backend/catalog/catalog.c                   |  6 +-
src/backend/catalog/storage.c                   |  4 +-
src/backend/replication/logical/reorderbuffer.c |  4 +-
src/backend/storage/buffer/bufmgr.c             | 48 ++++++---------
src/backend/storage/buffer/localbuf.c           |  6 +-
src/backend/storage/smgr/md.c                   | 59 ++++++++-----------
src/backend/utils/adt/dbsize.c                  | 10 ++--
src/bin/pg_rewind/filemap.c                     |  7 +--
src/common/relpath.c                            | 77 ++++++++++++++-----------
src/include/common/relpath.h                    | 52 ++++++++++++++++-
src/test/regress/expected/misc_functions.out    | 11 ++++
src/test/regress/regress.c                      | 29 ++++++++++
src/test/regress/sql/misc_functions.sql         |  8 +++
src/tools/pgindent/typedefs.list                |  1 +
18 files changed, 218 insertions(+), 159 deletions(-)



Message-ID: <E1tmvqL-0001EL-0K@gemulon.postgresql.org>
Permalink:  ../E1tmvqL-0001EL-0K@gemulon.postgresql.org/
Also on:    postgresql.org/message-id/E1tmvqL-0001EL-0K@gemulon.postgresql.org

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: andres@anarazel.de, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: Change relpath() et al to return path by value
  In-Reply-To: <E1tmvqL-0001EL-0K@gemulon.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox