Re: Should we say "wal_level = logical" instead of "wal_level >= logical"
От | Ashutosh Bapat |
---|---|
Тема | Re: Should we say "wal_level = logical" instead of "wal_level >= logical" |
Дата | |
Msg-id | CAExHW5spvrnRX2xTV2U-CxvAt3uJiowekYWfaWVHqixzgFqC0A@mail.gmail.com обсуждение исходный текст |
Ответ на | Should we say "wal_level = logical" instead of "wal_level >= logical" (Peter Smith <smithpb2250@gmail.com>) |
Список | pgsql-hackers |
On Thu, Oct 16, 2025 at 5:36 AM Peter Smith <smithpb2250@gmail.com> wrote: > > Hi Hackers, > > While reviewing another LR thread [1] it saw multiple wal_level > messages that seem more complex than necessary. > > Background: There are three wal_level values [2], ranked as: > minimal < replica < logical > > Notice there is no wal_level "above" logical. Despite this, there are > a number of places in the code and messages that say >= logical > instead of just = logical. Here is a list: > > heapam.c > - * This is only used in wal_level >= WAL_LEVEL_LOGICAL, and only for catalog > > xlog.c > - * This returns true if wal_level >= logical and we are inside a valid > > tablecmds.c > - /* should only get here if wal_level >= logical */ > > slot.c > - appendStringInfoString(&err_detail, _("Logical decoding on standby > requires \"wal_level\" >= \"logical\" on the primary server.")); > > decode.c > - errmsg("logical decoding on standby requires \"wal_level\" >= > \"logical\" on the primary"))); > > logical.c > - errmsg("logical decoding requires \"wal_level\" >= \"logical\""))); > - errmsg("logical decoding on standby requires \"wal_level\" >= > \"logical\" on the primary"))); > > slotsync.c > - * Logical slot sync/creation requires wal_level >= logical. > - errmsg("replication slot synchronization requires \"wal_level\" >= > \"logical\"")); > > standby.c > - if (wal_level >= WAL_LEVEL_LOGICAL && isCatalogRel) > - if (wal_level >= WAL_LEVEL_LOGICAL) > pg_createsubscriber.c > - pg_log_error("publisher requires \"wal_level\" >= \"logical\""); > > xlog.h > - #define XLogLogicalInfoActive() (wal_level >= WAL_LEVEL_LOGICAL) > > 035_standby_logical_decoding.pl > - # We are not able to read from the slot as it requires wal_level >= > logical on the primary server > - "logical decoding on standby requires \"wal_level\" >= \"logical\" > on the primary" > > ~~~ > > IMO all these error messages appear more complicated than necessary. > > Isn't it simpler to say: > "requires \"wal_level\" = \"logical\" ..." > > Instead of: > "requires \"wal_level\" >= \"logical\" ..." > > ~~~ > > FYI, there is other code that does not account for anything "above" > logical. e.g. > if (wal_level != WAL_LEVEL_LOGICAL) > > ~~~ > > I propose to write a patch to change all those user-facing messages. > > What about changing the code/comments as well? > > Thoughts? I think the reason it's phrased like that is because wal_level is cumulative - higher wal_levels are assumed to cover all the functions (and information) covered by lower wal levels. By phrasing it as wal_level >= logical we are being future-proof. If we add another level, we won't need to change documentation or error messages. But you are right it might confuse users because they won't see anything higher than logical. [1] is possibly changing that property of wal_level values or at least threatening to change it and we don't know what the next wal_level would be and whether it will continue to cover lower levels. But maybe users are already used to that phrasing since it seems be there for some time and without complaints. Did we always use that wording for example, did pre-logical wal_level messages also used wal_level >= replica? -- Best Wishes, Ashutosh Bapat
В списке pgsql-hackers по дате отправления: