From 8587040c871f2b4ad045c9d23c6cd8f23318e6a5 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Sun, 8 Aug 2021 16:45:20 +1000 Subject: [PATCH v12] Simplify recent parse_subscription_option changes. Make the parse_subscription_options function reponsible for zapping the SubOpts param up-front, instead of hoping the caller may do it. Remove redundant condition checks for "supported_opts" where we already know the option MUST be supported. --- src/backend/commands/subscriptioncmds.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 5157f44..796f823 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -94,8 +94,6 @@ static void ReportSlotConnectionError(List *rstates, Oid subid, char *slotname, * * Since not all options can be specified in both commands, this function * will report an error if mutually exclusive options are specified. - * - * Caller is expected to have cleared 'opts'. */ static void parse_subscription_options(ParseState *pstate, List *stmt_options, @@ -103,6 +101,9 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, { ListCell *lc; + /* Start out with cleared opts. */ + memset(opts, 0, sizeof(SubOpts)); + /* caller must expect some option */ Assert(supported_opts != 0); @@ -261,7 +262,6 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, { /* Check for incompatible options from the user. */ if (opts->enabled && - IsSet(supported_opts, SUBOPT_ENABLED) && IsSet(opts->specified_opts, SUBOPT_ENABLED)) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -270,7 +270,6 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, "connect = false", "enabled = true"))); if (opts->create_slot && - IsSet(supported_opts, SUBOPT_CREATE_SLOT) && IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT)) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -278,7 +277,6 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, "connect = false", "create_slot = true"))); if (opts->copy_data && - IsSet(supported_opts, SUBOPT_COPY_DATA) && IsSet(opts->specified_opts, SUBOPT_COPY_DATA)) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -296,11 +294,9 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, * was used. */ if (!opts->slot_name && - IsSet(supported_opts, SUBOPT_SLOT_NAME) && IsSet(opts->specified_opts, SUBOPT_SLOT_NAME)) { if (opts->enabled && - IsSet(supported_opts, SUBOPT_ENABLED) && IsSet(opts->specified_opts, SUBOPT_ENABLED)) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -309,7 +305,6 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, "slot_name = NONE", "enabled = true"))); if (opts->create_slot && - IsSet(supported_opts, SUBOPT_CREATE_SLOT) && IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT)) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -317,18 +312,14 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, errmsg("%s and %s are mutually exclusive options", "slot_name = NONE", "create_slot = true"))); - if (opts->enabled && - IsSet(supported_opts, SUBOPT_ENABLED) && - !IsSet(opts->specified_opts, SUBOPT_ENABLED)) + if (opts->enabled) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), /*- translator: both %s are strings of the form "option = value" */ errmsg("subscription with %s must also set %s", "slot_name = NONE", "enabled = false"))); - if (opts->create_slot && - IsSet(supported_opts, SUBOPT_CREATE_SLOT) && - !IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT)) + if (opts->create_slot) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), /*- translator: both %s are strings of the form "option = value" */ -- 1.8.3.1