Re: define PG_REPLSLOT_DIR

Hi,

On Tue, Aug 20, 2024 at 10:15:44AM -0400, Alvaro Herrera wrote:
> On 2024-Aug-19, Bertrand Drouvot wrote:
>
> > diff --git a/src/include/common/relpath.h b/src/include/common/relpath.h
> > index 6f006d5a93..a6cb091635 100644
> > --- a/src/include/common/relpath.h
> > +++ b/src/include/common/relpath.h
> > @@ -33,6 +33,10 @@ typedef Oid RelFileNumber;
> > #define TABLESPACE_VERSION_DIRECTORY "PG_" PG_MAJORVERSION "_" \
> > CppAsString2(CATALOG_VERSION_NO)
> >
> > +#define PG_TBLSPC_DIR "pg_tblspc"
>
> This one is missing some commentary along the lines of "This must not be
> changed, unless you want to break every tool in the universe". As is,
> it's quite tempting.

Yeah, makes sense, thanks.

> > +#define PG_TBLSPC_DIR_SLASH PG_TBLSPC_DIR "/"
>
> I would make this simply "pg_tblspc/", since it's not really possible to
> change pg_tblspc anyway. Also, have a comment explaining why we have
> it.

Please find attached v3 that:

- takes care of your comments (and also removed the use of PG_TBLSPC_DIR in
RELATIVE_PG_TBLSPC_DIR).
- removes the new macros from the comments (see Michael's and Yugo-San's
comments in [0] resp. [1]).
- adds a missing sizeof() (see [1]).
- implements Ashutosh's idea of adding a new SLOT_DIRNAME_ARGS (see [2]). It's
done in 0002 (I used REPLSLOT_DIR_ARGS though).
- fixed a macro usage in ReorderBufferCleanupSerializedTXNs() (was not at the
right location, discovered while implementing 0002).

[0]: https://www.postgresql.org/message-id/ZsRYPcOtoqbWzjGG%40paquier.xyz
[1]: https://www.postgresql.org/message-id/20240820213048.207aade6a75e0dc1fe4d1067%40sraoss.co.jp
[2]: https://www.postgresql.org/message-id/CAExHW5vkjxuvyQ1fPPnuDW4nAT5jqox09ie36kciOV2%2BrhjbHA%40mail.gmail.com

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com