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
