From d5fb6ef3324a215f7d7a5019103adac4c788df0f Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 18 Feb 2025 20:39:11 -0500 Subject: [PATCH v1] relpath-by-value --- src/include/common/relpath.h | 40 +++++ src/common/relpath.c | 162 +++++++++++-------- src/backend/storage/buffer/bufmgr.c | 39 ++--- src/test/regress/expected/misc_functions.out | 13 ++ src/test/regress/regress.c | 14 ++ src/test/regress/sql/misc_functions.sql | 10 ++ src/tools/pgindent/typedefs.list | 1 + 7 files changed, 183 insertions(+), 96 deletions(-) diff --git a/src/include/common/relpath.h b/src/include/common/relpath.h index 5162362e7d8..9e06ecceba5 100644 --- a/src/include/common/relpath.h +++ b/src/include/common/relpath.h @@ -77,11 +77,51 @@ extern PGDLLIMPORT const char *const forkNames[]; extern ForkNumber forkname_to_number(const char *forkName); extern int forkname_chars(const char *str, ForkNumber *fork); +/* + * The longest possible relation path lengths is from the following format: + * sprintf(rp.path, "%s/%u/%s/%u/t%d_%u", + * PG_TBLSPC_DIR, spcOid, + * TABLESPACE_VERSION_DIRECTORY, + * dbOid, procNumber, relNumber); + */ +#define REL_PATH_STRING_MAXLEN \ + ( \ + sizeof(PG_TBLSPC_DIR) - 1 \ + + sizeof((char)'/') \ + + OIDCHARS /* spcOid */ \ + + sizeof((char)'/') \ + + sizeof(TABLESPACE_VERSION_DIRECTORY) - 1 \ + + sizeof((char)'/') \ + + OIDCHARS /* dbOid */ \ + + sizeof((char)'/') \ + + sizeof((char)'t') /* temporary table indicator */ \ + + 6 /* procNumber */ \ + + sizeof((char)'_') \ + + OIDCHARS /* relNumber */ \ + + sizeof((char)'_') \ + + FORKNAMECHARS /* forkNames[forkNumber] */ \ + ) + +typedef struct RelPathString +{ + char path[REL_PATH_STRING_MAXLEN + 1]; +} RelPathString; + /* * Stuff for computing filesystem pathnames for relations. */ extern char *GetDatabasePath(Oid dbOid, Oid spcOid); +extern RelPathString RelationPathFor(Oid dbOid, Oid spcOid, RelFileNumber relNumber, + int procNumber, ForkNumber forkNumber); +#define relpathforperm(rlocator, forknum) \ + RelationPathFor((rlocator).dbOid, (rlocator).spcOid, (rlocator).relNumber, \ + INVALID_PROC_NUMBER, forknum) +#define relpathforbackend(rlocator, backend, forknum) \ + RelationPathFor((rlocator).dbOid, (rlocator).spcOid, (rlocator).relNumber, \ + backend, forknum) + + extern char *GetRelationPath(Oid dbOid, Oid spcOid, RelFileNumber relNumber, int procNumber, ForkNumber forkNumber); diff --git a/src/common/relpath.c b/src/common/relpath.c index 186156a9e44..55c7aa1f11a 100644 --- a/src/common/relpath.c +++ b/src/common/relpath.c @@ -129,6 +129,96 @@ GetDatabasePath(Oid dbOid, Oid spcOid) } } +/* + * RelationPathFor - construct path to a relation's file + * + * The result is returned in-place as a struct, to make it suitable for use in + * critical sections etc. + * + * See also GetRelationPath() + */ +RelPathString +RelationPathFor(Oid dbOid, Oid spcOid, RelFileNumber relNumber, + int procNumber, ForkNumber forkNumber) +{ + RelPathString rp; + + if (spcOid == GLOBALTABLESPACE_OID) + { + /* Shared system relations live in {datadir}/global */ + Assert(dbOid == 0); + Assert(procNumber == INVALID_PROC_NUMBER); + if (forkNumber != MAIN_FORKNUM) + sprintf(rp.path, "global/%u_%s", + relNumber, forkNames[forkNumber]); + else + sprintf(rp.path, "global/%u", + relNumber); + } + else if (spcOid == DEFAULTTABLESPACE_OID) + { + /* The default tablespace is {datadir}/base */ + if (procNumber == INVALID_PROC_NUMBER) + { + if (forkNumber != MAIN_FORKNUM) + { + sprintf(rp.path, "base/%u/%u_%s", + dbOid, relNumber, + forkNames[forkNumber]); + } + else + sprintf(rp.path, "base/%u/%u", + dbOid, relNumber); + } + else + { + if (forkNumber != MAIN_FORKNUM) + sprintf(rp.path, "base/%u/t%d_%u_%s", + dbOid, procNumber, relNumber, + forkNames[forkNumber]); + else + sprintf(rp.path, "base/%u/t%d_%u", + dbOid, procNumber, relNumber); + } + } + else + { + /* All other tablespaces are accessed via symlinks */ + if (procNumber == INVALID_PROC_NUMBER) + { + if (forkNumber != MAIN_FORKNUM) + sprintf(rp.path, "%s/%u/%s/%u/%u_%s", + PG_TBLSPC_DIR, spcOid, + TABLESPACE_VERSION_DIRECTORY, + dbOid, relNumber, + forkNames[forkNumber]); + else + sprintf(rp.path, "%s/%u/%s/%u/%u", + PG_TBLSPC_DIR, spcOid, + TABLESPACE_VERSION_DIRECTORY, + dbOid, relNumber); + } + else + { + if (forkNumber != MAIN_FORKNUM) + sprintf(rp.path, "%s/%u/%s/%u/t%d_%u_%s", + PG_TBLSPC_DIR, spcOid, + TABLESPACE_VERSION_DIRECTORY, + dbOid, procNumber, relNumber, + forkNames[forkNumber]); + else + sprintf(rp.path, "%s/%u/%s/%u/t%d_%u", + PG_TBLSPC_DIR, spcOid, + TABLESPACE_VERSION_DIRECTORY, + dbOid, procNumber, relNumber); + } + } + + Assert(strnlen(rp.path, REL_PATH_STRING_MAXLEN + 1) <= REL_PATH_STRING_MAXLEN); + + return rp; +} + /* * GetRelationPath - construct path to a relation's file * @@ -142,74 +232,6 @@ char * GetRelationPath(Oid dbOid, Oid spcOid, RelFileNumber relNumber, int procNumber, ForkNumber forkNumber) { - char *path; - - if (spcOid == GLOBALTABLESPACE_OID) - { - /* Shared system relations live in {datadir}/global */ - Assert(dbOid == 0); - Assert(procNumber == INVALID_PROC_NUMBER); - if (forkNumber != MAIN_FORKNUM) - path = psprintf("global/%u_%s", - relNumber, forkNames[forkNumber]); - else - path = psprintf("global/%u", relNumber); - } - else if (spcOid == DEFAULTTABLESPACE_OID) - { - /* The default tablespace is {datadir}/base */ - if (procNumber == INVALID_PROC_NUMBER) - { - if (forkNumber != MAIN_FORKNUM) - path = psprintf("base/%u/%u_%s", - dbOid, relNumber, - forkNames[forkNumber]); - else - path = psprintf("base/%u/%u", - dbOid, relNumber); - } - else - { - if (forkNumber != MAIN_FORKNUM) - path = psprintf("base/%u/t%d_%u_%s", - dbOid, procNumber, relNumber, - forkNames[forkNumber]); - else - path = psprintf("base/%u/t%d_%u", - dbOid, procNumber, relNumber); - } - } - else - { - /* All other tablespaces are accessed via symlinks */ - if (procNumber == INVALID_PROC_NUMBER) - { - if (forkNumber != MAIN_FORKNUM) - path = psprintf("%s/%u/%s/%u/%u_%s", - PG_TBLSPC_DIR, spcOid, - TABLESPACE_VERSION_DIRECTORY, - dbOid, relNumber, - forkNames[forkNumber]); - else - path = psprintf("%s/%u/%s/%u/%u", - PG_TBLSPC_DIR, spcOid, - TABLESPACE_VERSION_DIRECTORY, - dbOid, relNumber); - } - else - { - if (forkNumber != MAIN_FORKNUM) - path = psprintf("%s/%u/%s/%u/t%d_%u_%s", - PG_TBLSPC_DIR, spcOid, - TABLESPACE_VERSION_DIRECTORY, - dbOid, procNumber, relNumber, - forkNames[forkNumber]); - else - path = psprintf("%s/%u/%s/%u/t%d_%u", - PG_TBLSPC_DIR, spcOid, - TABLESPACE_VERSION_DIRECTORY, - dbOid, procNumber, relNumber); - } - } - return path; + return pstrdup(RelationPathFor(dbOid, spcOid, relNumber, + procNumber, forkNumber).path); } diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 75cfc2b6fe9..5275c06fc2f 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -3663,7 +3663,6 @@ DebugPrintBufferRefcount(Buffer buffer) { BufferDesc *buf; int32 loccount; - char *path; char *result; ProcNumber backend; uint32 buf_state; @@ -3683,15 +3682,14 @@ DebugPrintBufferRefcount(Buffer buffer) } /* theoretically we should lock the bufhdr here */ - path = relpathbackend(BufTagGetRelFileLocator(&buf->tag), backend, - BufTagGetForkNum(&buf->tag)); buf_state = pg_atomic_read_u32(&buf->state); result = psprintf("[%03d] (rel=%s, blockNum=%u, flags=0x%x, refcount=%u %d)", - buffer, path, + buffer, + relpathforbackend(BufTagGetRelFileLocator(&buf->tag), backend, + BufTagGetForkNum(&buf->tag)).path, buf->tag.blockNum, buf_state & BUF_FLAG_MASK, BUF_STATE_GET_REFCOUNT(buf_state), loccount); - pfree(path); return result; } @@ -5611,16 +5609,13 @@ AbortBufferIO(Buffer buffer) if (buf_state & BM_IO_ERROR) { /* Buffer is pinned, so we can read tag without spinlock */ - char *path; - - path = relpathperm(BufTagGetRelFileLocator(&buf_hdr->tag), - BufTagGetForkNum(&buf_hdr->tag)); ereport(WARNING, (errcode(ERRCODE_IO_ERROR), errmsg("could not write block %u of %s", - buf_hdr->tag.blockNum, path), + buf_hdr->tag.blockNum, + relpathforperm(BufTagGetRelFileLocator(&buf_hdr->tag), + BufTagGetForkNum(&buf_hdr->tag)).path), errdetail("Multiple failures --- write error might be permanent."))); - pfree(path); } } @@ -5637,14 +5632,10 @@ shared_buffer_write_error_callback(void *arg) /* Buffer is pinned, so we can read the tag without locking the spinlock */ if (bufHdr != NULL) - { - char *path = relpathperm(BufTagGetRelFileLocator(&bufHdr->tag), - BufTagGetForkNum(&bufHdr->tag)); - errcontext("writing block %u of relation %s", - bufHdr->tag.blockNum, path); - pfree(path); - } + bufHdr->tag.blockNum, + relpathforperm(BufTagGetRelFileLocator(&bufHdr->tag), + BufTagGetForkNum(&bufHdr->tag)).path); } /* @@ -5656,15 +5647,11 @@ local_buffer_write_error_callback(void *arg) BufferDesc *bufHdr = (BufferDesc *) arg; if (bufHdr != NULL) - { - char *path = relpathbackend(BufTagGetRelFileLocator(&bufHdr->tag), - MyProcNumber, - BufTagGetForkNum(&bufHdr->tag)); - errcontext("writing block %u of relation %s", - bufHdr->tag.blockNum, path); - pfree(path); - } + bufHdr->tag.blockNum, + relpathforbackend(BufTagGetRelFileLocator(&bufHdr->tag), + MyProcNumber, + BufTagGetForkNum(&bufHdr->tag)).path); } /* diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out index 106dedb519a..64e4d757ebf 100644 --- a/src/test/regress/expected/misc_functions.out +++ b/src/test/regress/expected/misc_functions.out @@ -903,3 +903,16 @@ SELECT gist_stratnum_common(3); 18 (1 row) +-- relpath tests +CREATE FUNCTION test_relpath() + RETURNS text + AS :'regresslib' + LANGUAGE C; +-- FIXME: This would include version number, which we don't want in the +-- expected file. +SELECT test_relpath(); + test_relpath +------------------------------------------------------------------------- + pg_tblspc/4294967295/PG_18_202502112/4294967295/t262142_4294967295_init +(1 row) + diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c index 667d9835148..b8d2a1725fe 100644 --- a/src/test/regress/regress.c +++ b/src/test/regress/regress.c @@ -1208,3 +1208,17 @@ binary_coercible(PG_FUNCTION_ARGS) PG_RETURN_BOOL(IsBinaryCoercible(srctype, targettype)); } + +#include "postmaster/postmaster.h" + +PG_FUNCTION_INFO_V1(test_relpath); +Datum +test_relpath(PG_FUNCTION_ARGS) +{ + RelPathString rpath; + + rpath = RelationPathFor(OID_MAX, OID_MAX, OID_MAX, MAX_BACKENDS - 1, + INIT_FORKNUM); + + PG_RETURN_DATUM(CStringGetTextDatum(rpath.path)); +} diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql index 753a0f41c03..e7a2e4fb7c2 100644 --- a/src/test/regress/sql/misc_functions.sql +++ b/src/test/regress/sql/misc_functions.sql @@ -403,3 +403,13 @@ DROP FUNCTION explain_mask_costs(text, bool, bool, bool, bool); -- test stratnum support functions SELECT gist_stratnum_common(7); SELECT gist_stratnum_common(3); + + +-- relpath tests +CREATE FUNCTION test_relpath() + RETURNS text + AS :'regresslib' + LANGUAGE C; +-- FIXME: This would include version number, which we don't want in the +-- expected file. +SELECT test_relpath(); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 80aa50d55a4..ce75ce75e8f 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2402,6 +2402,7 @@ RelMapFile RelMapping RelOptInfo RelOptKind +RelPathString RelToCheck RelToCluster RelabelType -- 2.48.1.76.g4e746b1a31.dirty