Hi, On Tue, Feb 27, 2024 at 06:17:44PM +1100, Peter Smith wrote: > +static bool > +validate_standby_slots(char **newval) > +{ > + char *rawname; > + List *elemlist; > + ListCell *lc; > + bool ok; > + > + /* Need a modifiable copy of string */ > + rawname = pstrdup(*newval); > + > + /* Verify syntax and parse string into a list of identifiers */ > + ok = SplitIdentifierString(rawname, ',', &elemlist); > + > + if (!ok) > + { > + GUC_check_errdetail("List syntax is invalid."); > + } > + > + /* > + * If the replication slots' data have been initialized, verify if the > + * specified slots exist and are logical slots. > + */ > + else if (ReplicationSlotCtl) > + { > + foreach(lc, elemlist) > > 6a. > So, if the ReplicationSlotCtl is NULL, it is possible to return > ok=true without ever checking if the slots exist or are of the correct > kind. I am wondering what are the ramifications of that. -- e.g. > assuming names are OK when maybe they aren't OK at all. AFAICT this > works because it relies on getting subsequent WARNINGS when calling > FilterStandbySlots(). If that is correct then maybe the comment here > can be enhanced to say so. > > Indeed, if it works like that, now I am wondering do we need this for > loop validation at all. e.g. it seems just a matter of timing whether > we get ERRORs validating the GUC here, or WARNINGS later in the > FilterStandbySlots. Maybe we don't need the double-checking and it is > enough to check in FilterStandbySlots?
Good point, I have the feeling that it is enough to check in FilterStandbySlots(). Indeed, if the value is syntactically correct, then I think that its actual value "really" matters when the logical decoding is starting/running, does it provide additional benefits "before" that? Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com