On 2024-Aug-14, Bertrand Drouvot wrote:
> Out of curiosity, checking the sizes of affected files (O2, no debug):
>
> with patch:
>
> text data bss dec hex filename
> 30304 0 8 30312 7668 src/backend/replication/logical/reorderbuffer.o
> without patch:
> 30286 0 8 30294 7656 src/backend/replication/logical/reorderbuffer.o
Hmm, I suppose this delta can be explained because because the literal
string is used inside larger snprintf() format strings or similar, so
things like
snprintf(path, sizeof(path),
- "pg_replslot/%s/%s", slotname,
+ "%s/%s/%s", PG_REPLSLOT_DIR, slotname,
spill_de->d_name);
and
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not remove file \"%s\" during removal of pg_replslot/%s/xid*: %m",
- path, slotname)));
+ errmsg("could not remove file \"%s\" during removal of %s/%s/xid*: %m",
+ PG_REPLSLOT_DIR, path, slotname)));
I don't disagree with making this change, but changing some of the other
directory names you suggest, such as
> pg_notify
> pg_serial
> pg_subtrans
> pg_multixact
> pg_wal
would probably make no difference, since there are no literal strings
that contain that as a substring(*). It might some sense to handle
pg_tblspc similarly. As for pg_logical, I think you'll want separate
defines for pg_logical/mappings, pg_logical/snapshots and so on.
(*) I hope you're not going to suggest this kind of change (word-diff):
if (GetOldestSnapshot())
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("[-pg_wal-]{+%s+}_replay_wait() must be only called without an active or registered
snapshot"{+,PG_WAL_DIR+}),
errdetail("Make sure [-pg_wal-]{+%s+}_replay_wait() isn't called within a transaction with an
isolationlevel higher than READ COMMITTED, another procedure, or a function."{+, PG_WAL_DIR+})));
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Los trabajadores menos efectivos son sistematicamente llevados al lugar
donde pueden hacer el menor daño posible: gerencia." (El principio Dilbert)
